[PATCH V5 0/7] 52-bit userspace VAs

[PATCH V5 0/7] 52-bit userspace VAs

[Steve Capper]

This patch series brings support for 52-bit userspace VAs to systems that
have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
PAGE_SIZE.

If no hardware support is present, the kernel runs with a 48-bit VA space
for userspace.

Userspace can exploit this feature by providing an address hint to mmap
where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
same way as a 48-bit VA system (this is to maintain compatibility with
software that assumes the maximum VA size on arm64 is 48-bit).

This patch series applies to 4.20-rc1.

Testing was in a model with Trusted Firmware and UEFI for boot.

Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
check for VA space support mismatch between CPUs.

Changed in V4, pgd_index changes dropped in favour of offsetting the
ttbr1. This is performed in a new patch, #4.

Changed in V3, COMPAT fixes added (and tested with 32-bit userspace code).
Extra patch added to allow forcing all userspace allocations to come from
52-bits (to allow for debugging and testing).

The major change to V2 of the series is that mm/mmap.c is altered in the
first patch of the series (rather than copied over to arch/arm64).


Steve Capper (7):
  mm: mmap: Allow for "high" userspace addresses
  arm64: mm: Introduce DEFAULT_MAP_WINDOW
  arm64: mm: Define arch_get_mmap_end, arch_get_mmap_base
  arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD
  arm64: mm: Prevent mismatched 52-bit VA support
  arm64: mm: introduce 52-bit userspace support
  arm64: mm: Allow forcing all userspace addresses to 52-bit

 arch/arm64/Kconfig                      | 17 +++++++++++
 arch/arm64/include/asm/assembler.h      | 30 ++++++++++++++++---
 arch/arm64/include/asm/elf.h            |  4 +++
 arch/arm64/include/asm/mmu_context.h    |  3 ++
 arch/arm64/include/asm/pgtable-hwdef.h  |  9 ++++++
 arch/arm64/include/asm/processor.h      | 36 ++++++++++++++++++----
 arch/arm64/kernel/head.S                | 40 +++++++++++++++++++++++++
 arch/arm64/kernel/hibernate-asm.S       |  1 +
 arch/arm64/kernel/smp.c                 |  5 ++++
 arch/arm64/mm/fault.c                   |  2 +-
 arch/arm64/mm/init.c                    |  2 +-
 arch/arm64/mm/mmu.c                     |  1 +
 arch/arm64/mm/proc.S                    | 14 ++++++++-
 drivers/firmware/efi/arm-runtime.c      |  2 +-
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 mm/mmap.c                               | 25 +++++++++++-----
 16 files changed, 171 insertions(+), 22 deletions(-)

-- 
2.19.2

[PATCH V5 0/7] 52-bit userspace VAs

[Steve Capper]

This patch series brings support for 52-bit userspace VAs to systems that
have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
PAGE_SIZE.

If no hardware support is present, the kernel runs with a 48-bit VA space
for userspace.

Userspace can exploit this feature by providing an address hint to mmap
where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
same way as a 48-bit VA system (this is to maintain compatibility with
software that assumes the maximum VA size on arm64 is 48-bit).

This patch series applies to 4.20-rc1.

Testing was in a model with Trusted Firmware and UEFI for boot.

Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
check for VA space support mismatch between CPUs.

Changed in V4, pgd_index changes dropped in favour of offsetting the
ttbr1. This is performed in a new patch, #4.

Changed in V3, COMPAT fixes added (and tested with 32-bit userspace code).
Extra patch added to allow forcing all userspace allocations to come from
52-bits (to allow for debugging and testing).

The major change to V2 of the series is that mm/mmap.c is altered in the
first patch of the series (rather than copied over to arch/arm64).


Steve Capper (7):
  mm: mmap: Allow for "high" userspace addresses
  arm64: mm: Introduce DEFAULT_MAP_WINDOW
  arm64: mm: Define arch_get_mmap_end, arch_get_mmap_base
  arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD
  arm64: mm: Prevent mismatched 52-bit VA support
  arm64: mm: introduce 52-bit userspace support
  arm64: mm: Allow forcing all userspace addresses to 52-bit

 arch/arm64/Kconfig                      | 17 +++++++++++
 arch/arm64/include/asm/assembler.h      | 30 ++++++++++++++++---
 arch/arm64/include/asm/elf.h            |  4 +++
 arch/arm64/include/asm/mmu_context.h    |  3 ++
 arch/arm64/include/asm/pgtable-hwdef.h  |  9 ++++++
 arch/arm64/include/asm/processor.h      | 36 ++++++++++++++++++----
 arch/arm64/kernel/head.S                | 40 +++++++++++++++++++++++++
 arch/arm64/kernel/hibernate-asm.S       |  1 +
 arch/arm64/kernel/smp.c                 |  5 ++++
 arch/arm64/mm/fault.c                   |  2 +-
 arch/arm64/mm/init.c                    |  2 +-
 arch/arm64/mm/mmu.c                     |  1 +
 arch/arm64/mm/proc.S                    | 14 ++++++++-
 drivers/firmware/efi/arm-runtime.c      |  2 +-
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 mm/mmap.c                               | 25 +++++++++++-----
 16 files changed, 171 insertions(+), 22 deletions(-)

-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 1/7] mm: mmap: Allow for "high" userspace addresses

[Steve Capper]

This patch adds support for "high" userspace addresses that are
optionally supported on the system and have to be requested via a hint
mechanism ("high" addr parameter to mmap).

Architectures such as powerpc and x86 achieve this by making changes to
their architectural versions of arch_get_unmapped_* functions. However,
on arm64 we use the generic versions of these functions.

Rather than duplicate the generic arch_get_unmapped_* implementations
for arm64, this patch instead introduces two architectural helper macros
and applies them to arch_get_unmapped_*:
 arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint
 arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint

If these macros are not defined in architectural code then they default
to (TASK_SIZE) and (base) so should not introduce any behavioural
changes to architectures that do not define them.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---

Changed in V4, added Catalin's reviewed by

Changed in V3, commit log cleared up, explanation given for why core
code change over just architectural change
---
 mm/mmap.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 6c04292e16a7..7bb64381e77c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2066,6 +2066,15 @@ unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
 	return gap_end;
 }
 
+
+#ifndef arch_get_mmap_end
+#define arch_get_mmap_end(addr)	(TASK_SIZE)
+#endif
+
+#ifndef arch_get_mmap_base
+#define arch_get_mmap_base(addr, base) (base)
+#endif
+
 /* Get an address range which is currently unmapped.
  * For shmat() with addr=0.
  *
@@ -2085,8 +2094,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma, *prev;
 	struct vm_unmapped_area_info info;
+	const unsigned long mmap_end = arch_get_mmap_end(addr);
 
-	if (len > TASK_SIZE - mmap_min_addr)
+	if (len > mmap_end - mmap_min_addr)
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED)
@@ -2095,7 +2105,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma_prev(mm, addr, &prev);
-		if (TASK_SIZE - len >= addr && addr >= mmap_min_addr &&
+		if (mmap_end - len >= addr && addr >= mmap_min_addr &&
 		    (!vma || addr + len <= vm_start_gap(vma)) &&
 		    (!prev || addr >= vm_end_gap(prev)))
 			return addr;
@@ -2104,7 +2114,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	info.flags = 0;
 	info.length = len;
 	info.low_limit = mm->mmap_base;
-	info.high_limit = TASK_SIZE;
+	info.high_limit = mmap_end;
 	info.align_mask = 0;
 	return vm_unmapped_area(&info);
 }
@@ -2124,9 +2134,10 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	struct mm_struct *mm = current->mm;
 	unsigned long addr = addr0;
 	struct vm_unmapped_area_info info;
+	const unsigned long mmap_end = arch_get_mmap_end(addr);
 
 	/* requested length too big for entire address space */
-	if (len > TASK_SIZE - mmap_min_addr)
+	if (len > mmap_end - mmap_min_addr)
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED)
@@ -2136,7 +2147,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma_prev(mm, addr, &prev);
-		if (TASK_SIZE - len >= addr && addr >= mmap_min_addr &&
+		if (mmap_end - len >= addr && addr >= mmap_min_addr &&
 				(!vma || addr + len <= vm_start_gap(vma)) &&
 				(!prev || addr >= vm_end_gap(prev)))
 			return addr;
@@ -2145,7 +2156,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
 	info.length = len;
 	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
-	info.high_limit = mm->mmap_base;
+	info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
 	info.align_mask = 0;
 	addr = vm_unmapped_area(&info);
 
@@ -2159,7 +2170,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 		VM_BUG_ON(addr != -ENOMEM);
 		info.flags = 0;
 		info.low_limit = TASK_UNMAPPED_BASE;
-		info.high_limit = TASK_SIZE;
+		info.high_limit = mmap_end;
 		addr = vm_unmapped_area(&info);
 	}
 
-- 
2.19.2

[PATCH V5 1/7] mm: mmap: Allow for "high" userspace addresses

[Steve Capper]

This patch adds support for "high" userspace addresses that are
optionally supported on the system and have to be requested via a hint
mechanism ("high" addr parameter to mmap).

Architectures such as powerpc and x86 achieve this by making changes to
their architectural versions of arch_get_unmapped_* functions. However,
on arm64 we use the generic versions of these functions.

Rather than duplicate the generic arch_get_unmapped_* implementations
for arm64, this patch instead introduces two architectural helper macros
and applies them to arch_get_unmapped_*:
 arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint
 arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint

If these macros are not defined in architectural code then they default
to (TASK_SIZE) and (base) so should not introduce any behavioural
changes to architectures that do not define them.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---

Changed in V4, added Catalin's reviewed by

Changed in V3, commit log cleared up, explanation given for why core
code change over just architectural change
---
 mm/mmap.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 6c04292e16a7..7bb64381e77c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2066,6 +2066,15 @@ unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
 	return gap_end;
 }
 
+
+#ifndef arch_get_mmap_end
+#define arch_get_mmap_end(addr)	(TASK_SIZE)
+#endif
+
+#ifndef arch_get_mmap_base
+#define arch_get_mmap_base(addr, base) (base)
+#endif
+
 /* Get an address range which is currently unmapped.
  * For shmat() with addr=0.
  *
@@ -2085,8 +2094,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma, *prev;
 	struct vm_unmapped_area_info info;
+	const unsigned long mmap_end = arch_get_mmap_end(addr);
 
-	if (len > TASK_SIZE - mmap_min_addr)
+	if (len > mmap_end - mmap_min_addr)
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED)
@@ -2095,7 +2105,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma_prev(mm, addr, &prev);
-		if (TASK_SIZE - len >= addr && addr >= mmap_min_addr &&
+		if (mmap_end - len >= addr && addr >= mmap_min_addr &&
 		    (!vma || addr + len <= vm_start_gap(vma)) &&
 		    (!prev || addr >= vm_end_gap(prev)))
 			return addr;
@@ -2104,7 +2114,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	info.flags = 0;
 	info.length = len;
 	info.low_limit = mm->mmap_base;
-	info.high_limit = TASK_SIZE;
+	info.high_limit = mmap_end;
 	info.align_mask = 0;
 	return vm_unmapped_area(&info);
 }
@@ -2124,9 +2134,10 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	struct mm_struct *mm = current->mm;
 	unsigned long addr = addr0;
 	struct vm_unmapped_area_info info;
+	const unsigned long mmap_end = arch_get_mmap_end(addr);
 
 	/* requested length too big for entire address space */
-	if (len > TASK_SIZE - mmap_min_addr)
+	if (len > mmap_end - mmap_min_addr)
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED)
@@ -2136,7 +2147,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
 		vma = find_vma_prev(mm, addr, &prev);
-		if (TASK_SIZE - len >= addr && addr >= mmap_min_addr &&
+		if (mmap_end - len >= addr && addr >= mmap_min_addr &&
 				(!vma || addr + len <= vm_start_gap(vma)) &&
 				(!prev || addr >= vm_end_gap(prev)))
 			return addr;
@@ -2145,7 +2156,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
 	info.length = len;
 	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
-	info.high_limit = mm->mmap_base;
+	info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
 	info.align_mask = 0;
 	addr = vm_unmapped_area(&info);
 
@@ -2159,7 +2170,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 		VM_BUG_ON(addr != -ENOMEM);
 		info.flags = 0;
 		info.low_limit = TASK_UNMAPPED_BASE;
-		info.high_limit = TASK_SIZE;
+		info.high_limit = mmap_end;
 		addr = vm_unmapped_area(&info);
 	}
 
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 2/7] arm64: mm: Introduce DEFAULT_MAP_WINDOW

[Steve Capper]

We wish to introduce a 52-bit virtual address space for userspace but
maintain compatibility with software that assumes the maximum VA space
size is 48 bit.

In order to achieve this, on 52-bit VA systems, we make mmap behave as
if it were running on a 48-bit VA system (unless userspace explicitly
requests a VA where addr[51:48] != 0).

On a system running a 52-bit userspace we need TASK_SIZE to represent
the 52-bit limit as it is used in various places to distinguish between
kernelspace and userspace addresses.

Thus we need a new limit for mmap, stack, ELF loader and EFI (which uses
TTBR0) to represent the non-extended VA space.

This patch introduces DEFAULT_MAP_WINDOW and DEFAULT_MAP_WINDOW_64 and
switches the appropriate logic to use that instead of TASK_SIZE.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

---

Changed in V3: corrections to allow COMPAT 32-bit EL0 mode to work
---
 arch/arm64/include/asm/elf.h            |  2 +-
 arch/arm64/include/asm/processor.h      | 10 ++++++++--
 arch/arm64/mm/init.c                    |  2 +-
 drivers/firmware/efi/arm-runtime.c      |  2 +-
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 433b9554c6a1..bc9bd9e77d9d 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -117,7 +117,7 @@
  * 64-bit, this is above 4GB to leave the entire 32-bit address
  * space open for things that want to use the area for 32-bit pointers.
  */
-#define ELF_ET_DYN_BASE		(2 * TASK_SIZE_64 / 3)
+#define ELF_ET_DYN_BASE		(2 * DEFAULT_MAP_WINDOW_64 / 3)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 3e2091708b8e..50586ca6bacb 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -45,19 +45,25 @@
  * TASK_SIZE - the maximum size of a user space task.
  * TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
  */
+
+#define DEFAULT_MAP_WINDOW_64	(UL(1) << VA_BITS)
+
 #ifdef CONFIG_COMPAT
 #define TASK_SIZE_32		UL(0x100000000)
 #define TASK_SIZE		(test_thread_flag(TIF_32BIT) ? \
 				TASK_SIZE_32 : TASK_SIZE_64)
 #define TASK_SIZE_OF(tsk)	(test_tsk_thread_flag(tsk, TIF_32BIT) ? \
 				TASK_SIZE_32 : TASK_SIZE_64)
+#define DEFAULT_MAP_WINDOW	(test_thread_flag(TIF_32BIT) ? \
+				TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64)
 #else
 #define TASK_SIZE		TASK_SIZE_64
+#define DEFAULT_MAP_WINDOW	DEFAULT_MAP_WINDOW_64
 #endif /* CONFIG_COMPAT */
 
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 4))
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#define STACK_TOP_MAX		DEFAULT_MAP_WINDOW_64
 
-#define STACK_TOP_MAX		TASK_SIZE_64
 #ifdef CONFIG_COMPAT
 #define AARCH32_VECTORS_BASE	0xffff0000
 #define STACK_TOP		(test_thread_flag(TIF_32BIT) ? \
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..7239c103be06 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -609,7 +609,7 @@ void __init mem_init(void)
 	 * detected at build time already.
 	 */
 #ifdef CONFIG_COMPAT
-	BUILD_BUG_ON(TASK_SIZE_32			> TASK_SIZE_64);
+	BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64);
 #endif
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 922cfb813109..952cec5b611a 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -38,7 +38,7 @@ static struct ptdump_info efi_ptdump_info = {
 	.mm		= &efi_mm,
 	.markers	= (struct addr_marker[]){
 		{ 0,		"UEFI runtime start" },
-		{ TASK_SIZE_64,	"UEFI runtime end" }
+		{ DEFAULT_MAP_WINDOW_64, "UEFI runtime end" }
 	},
 	.base_addr	= 0,
 };
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 30ac0c975f8a..d1ec7136e3e1 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -33,7 +33,7 @@
 #define EFI_RT_VIRTUAL_SIZE	SZ_512M
 
 #ifdef CONFIG_ARM64
-# define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE_64
+# define EFI_RT_VIRTUAL_LIMIT	DEFAULT_MAP_WINDOW_64
 #else
 # define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE
 #endif
-- 
2.19.2

[PATCH V5 2/7] arm64: mm: Introduce DEFAULT_MAP_WINDOW

[Steve Capper]

We wish to introduce a 52-bit virtual address space for userspace but
maintain compatibility with software that assumes the maximum VA space
size is 48 bit.

In order to achieve this, on 52-bit VA systems, we make mmap behave as
if it were running on a 48-bit VA system (unless userspace explicitly
requests a VA where addr[51:48] != 0).

On a system running a 52-bit userspace we need TASK_SIZE to represent
the 52-bit limit as it is used in various places to distinguish between
kernelspace and userspace addresses.

Thus we need a new limit for mmap, stack, ELF loader and EFI (which uses
TTBR0) to represent the non-extended VA space.

This patch introduces DEFAULT_MAP_WINDOW and DEFAULT_MAP_WINDOW_64 and
switches the appropriate logic to use that instead of TASK_SIZE.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

---

Changed in V3: corrections to allow COMPAT 32-bit EL0 mode to work
---
 arch/arm64/include/asm/elf.h            |  2 +-
 arch/arm64/include/asm/processor.h      | 10 ++++++++--
 arch/arm64/mm/init.c                    |  2 +-
 drivers/firmware/efi/arm-runtime.c      |  2 +-
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 433b9554c6a1..bc9bd9e77d9d 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -117,7 +117,7 @@
  * 64-bit, this is above 4GB to leave the entire 32-bit address
  * space open for things that want to use the area for 32-bit pointers.
  */
-#define ELF_ET_DYN_BASE		(2 * TASK_SIZE_64 / 3)
+#define ELF_ET_DYN_BASE		(2 * DEFAULT_MAP_WINDOW_64 / 3)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 3e2091708b8e..50586ca6bacb 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -45,19 +45,25 @@
  * TASK_SIZE - the maximum size of a user space task.
  * TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
  */
+
+#define DEFAULT_MAP_WINDOW_64	(UL(1) << VA_BITS)
+
 #ifdef CONFIG_COMPAT
 #define TASK_SIZE_32		UL(0x100000000)
 #define TASK_SIZE		(test_thread_flag(TIF_32BIT) ? \
 				TASK_SIZE_32 : TASK_SIZE_64)
 #define TASK_SIZE_OF(tsk)	(test_tsk_thread_flag(tsk, TIF_32BIT) ? \
 				TASK_SIZE_32 : TASK_SIZE_64)
+#define DEFAULT_MAP_WINDOW	(test_thread_flag(TIF_32BIT) ? \
+				TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64)
 #else
 #define TASK_SIZE		TASK_SIZE_64
+#define DEFAULT_MAP_WINDOW	DEFAULT_MAP_WINDOW_64
 #endif /* CONFIG_COMPAT */
 
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 4))
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#define STACK_TOP_MAX		DEFAULT_MAP_WINDOW_64
 
-#define STACK_TOP_MAX		TASK_SIZE_64
 #ifdef CONFIG_COMPAT
 #define AARCH32_VECTORS_BASE	0xffff0000
 #define STACK_TOP		(test_thread_flag(TIF_32BIT) ? \
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..7239c103be06 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -609,7 +609,7 @@ void __init mem_init(void)
 	 * detected at build time already.
 	 */
 #ifdef CONFIG_COMPAT
-	BUILD_BUG_ON(TASK_SIZE_32			> TASK_SIZE_64);
+	BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64);
 #endif
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 922cfb813109..952cec5b611a 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -38,7 +38,7 @@ static struct ptdump_info efi_ptdump_info = {
 	.mm		= &efi_mm,
 	.markers	= (struct addr_marker[]){
 		{ 0,		"UEFI runtime start" },
-		{ TASK_SIZE_64,	"UEFI runtime end" }
+		{ DEFAULT_MAP_WINDOW_64, "UEFI runtime end" }
 	},
 	.base_addr	= 0,
 };
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 30ac0c975f8a..d1ec7136e3e1 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -33,7 +33,7 @@
 #define EFI_RT_VIRTUAL_SIZE	SZ_512M
 
 #ifdef CONFIG_ARM64
-# define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE_64
+# define EFI_RT_VIRTUAL_LIMIT	DEFAULT_MAP_WINDOW_64
 #else
 # define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE
 #endif
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 3/7] arm64: mm: Define arch_get_mmap_end, arch_get_mmap_base

[Steve Capper]

Now that we have DEFAULT_MAP_WINDOW defined, we can arch_get_mmap_end
and arch_get_mmap_base helpers to allow for high addresses in mmap.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/processor.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 50586ca6bacb..fe95fd8b065e 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -72,6 +72,13 @@
 #define STACK_TOP		STACK_TOP_MAX
 #endif /* CONFIG_COMPAT */
 
+#define arch_get_mmap_end(addr) ((addr > DEFAULT_MAP_WINDOW) ? TASK_SIZE :\
+				DEFAULT_MAP_WINDOW)
+
+#define arch_get_mmap_base(addr, base) ((addr > DEFAULT_MAP_WINDOW) ? \
+					base + TASK_SIZE - DEFAULT_MAP_WINDOW :\
+					base)
+
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT	(arm64_dma_phys_limit - 1)
 
-- 
2.19.2

[PATCH V5 3/7] arm64: mm: Define arch_get_mmap_end, arch_get_mmap_base

[Steve Capper]

Now that we have DEFAULT_MAP_WINDOW defined, we can arch_get_mmap_end
and arch_get_mmap_base helpers to allow for high addresses in mmap.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/processor.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 50586ca6bacb..fe95fd8b065e 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -72,6 +72,13 @@
 #define STACK_TOP		STACK_TOP_MAX
 #endif /* CONFIG_COMPAT */
 
+#define arch_get_mmap_end(addr) ((addr > DEFAULT_MAP_WINDOW) ? TASK_SIZE :\
+				DEFAULT_MAP_WINDOW)
+
+#define arch_get_mmap_base(addr, base) ((addr > DEFAULT_MAP_WINDOW) ? \
+					base + TASK_SIZE - DEFAULT_MAP_WINDOW :\
+					base)
+
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT	(arm64_dma_phys_limit - 1)
 
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Steve Capper]

Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
entries (for the 48-bit case) to 1024 entries. This quantity,
PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
to a given virtual address, addr:

pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)

Userspace addresses are prefixed by 0's, so for a 48-bit userspace
address, uva, the following is true:
(uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)

In other words, a 48-bit userspace address will have the same pgd_index
when using PTRS_PER_PGD = 64 and 1024.

Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
kva, we have the following inequality:
(kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)

In other words a 48-bit kernel virtual address will have a different
pgd_index when using PTRS_PER_PGD = 64 and 1024.

If, however, we note that:
kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)

We can consider:
(kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
 = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
 = 0x3C0

In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).

For kernel configuration where 52-bit userspace VAs are possible, this
patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
52-bit value.

Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Steve Capper <steve.capper@arm.com>

---

Changed in V5, removed ttbr1 save/restore logic for software PAN as
hardware PAN is a mandatory ARMv8.1 feature anyway. The logic to enable
52-bit VAs has also been changed to depend on
ARM64_PAN || !ARM64_SW_TTBR0_PAN
(in a later patch)

This patch is new in V4 of the series
---
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/pgtable-hwdef.h |  9 +++++++++
 arch/arm64/kernel/head.S               |  1 +
 arch/arm64/kernel/hibernate-asm.S      |  1 +
 arch/arm64/mm/proc.S                   |  4 ++++
 5 files changed, 38 insertions(+)

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 6142402c2eb4..e2fe378d2a63 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -515,6 +515,29 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
 	mrs	\rd, sp_el0
 	.endm
 
+/*
+ * Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD.
+ * orr is used as it can cover the immediate value (and is idempotent).
+ * In future this may be nop'ed out when dealing with 52-bit kernel VAs.
+ * 	ttbr: Value of ttbr to set, modified.
+ */
+	.macro	offset_ttbr1, ttbr
+#ifdef CONFIG_ARM64_52BIT_VA
+	orr	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
+#endif
+	.endm
+
+/*
+ * Perform the reverse of offset_ttbr1.
+ * bic is used as it can cover the immediate value and, in future, won't need
+ * to be nop'ed out when dealing with 52-bit kernel VAs.
+ */
+	.macro	restore_ttbr1, ttbr
+#ifdef CONFIG_ARM64_52BIT_VA
+	bic	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
+#endif
+	.endm
+
 /*
  * Arrange a physical address in a TTBR register, taking care of 52-bit
  * addresses.
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 1d7d8da2ef9b..4a29c7e03ae4 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -80,7 +80,11 @@
 #define PGDIR_SHIFT		ARM64_HW_PGTABLE_LEVEL_SHIFT(4 - CONFIG_PGTABLE_LEVELS)
 #define PGDIR_SIZE		(_AC(1, UL) << PGDIR_SHIFT)
 #define PGDIR_MASK		(~(PGDIR_SIZE-1))
+#ifdef CONFIG_ARM64_52BIT_VA
+#define PTRS_PER_PGD		(1 << (52 - PGDIR_SHIFT))
+#else
 #define PTRS_PER_PGD		(1 << (VA_BITS - PGDIR_SHIFT))
+#endif
 
 /*
  * Section address mask and size definitions.
@@ -306,4 +310,9 @@
 #define TTBR_BADDR_MASK_52	(((UL(1) << 46) - 1) << 2)
 #endif
 
+#ifdef CONFIG_ARM64_52BIT_VA
+#define TTBR1_BADDR_4852_OFFSET	(((UL(1) << (52 - PGDIR_SHIFT)) - \
+				 (UL(1) << (48 - PGDIR_SHIFT))) * 8)
+#endif
+
 #endif
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 4471f570a295..f60081be9a1b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -769,6 +769,7 @@ ENTRY(__enable_mmu)
 	phys_to_ttbr x1, x1
 	phys_to_ttbr x2, x2
 	msr	ttbr0_el1, x2			// load TTBR0
+	offset_ttbr1 x1
 	msr	ttbr1_el1, x1			// load TTBR1
 	isb
 	msr	sctlr_el1, x0
diff --git a/arch/arm64/kernel/hibernate-asm.S b/arch/arm64/kernel/hibernate-asm.S
index dd14ab8c9f72..fe36d85c60bd 100644
--- a/arch/arm64/kernel/hibernate-asm.S
+++ b/arch/arm64/kernel/hibernate-asm.S
@@ -40,6 +40,7 @@
 	tlbi	vmalle1
 	dsb	nsh
 	phys_to_ttbr \tmp, \page_table
+	offset_ttbr1 \tmp
 	msr	ttbr1_el1, \tmp
 	isb
 .endm
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 2c75b0b903ae..2db1c491d45d 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -182,6 +182,7 @@ ENDPROC(cpu_do_switch_mm)
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
 	phys_to_ttbr \tmp2, \tmp1
+	offset_ttbr1 \tmp2
 	msr	ttbr1_el1, \tmp2
 	isb
 	tlbi	vmalle1
@@ -200,6 +201,7 @@ ENTRY(idmap_cpu_replace_ttbr1)
 
 	__idmap_cpu_set_reserved_ttbr1 x1, x3
 
+	offset_ttbr1 x0
 	msr	ttbr1_el1, x0
 	isb
 
@@ -254,6 +256,7 @@ ENTRY(idmap_kpti_install_ng_mappings)
 	pte		.req	x16
 
 	mrs	swapper_ttb, ttbr1_el1
+	restore_ttbr1	swapper_ttb
 	adr	flag_ptr, __idmap_kpti_flag
 
 	cbnz	cpu, __idmap_kpti_secondary
@@ -373,6 +376,7 @@ __idmap_kpti_secondary:
 	cbnz	w18, 1b
 
 	/* All done, act like nothing happened */
+	offset_ttbr1 swapper_ttb
 	msr	ttbr1_el1, swapper_ttb
 	isb
 	ret
-- 
2.19.2

[PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Steve Capper]

Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
entries (for the 48-bit case) to 1024 entries. This quantity,
PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
to a given virtual address, addr:

pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)

Userspace addresses are prefixed by 0's, so for a 48-bit userspace
address, uva, the following is true:
(uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)

In other words, a 48-bit userspace address will have the same pgd_index
when using PTRS_PER_PGD = 64 and 1024.

Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
kva, we have the following inequality:
(kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)

In other words a 48-bit kernel virtual address will have a different
pgd_index when using PTRS_PER_PGD = 64 and 1024.

If, however, we note that:
kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)

We can consider:
(kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
 = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
 = 0x3C0

In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).

For kernel configuration where 52-bit userspace VAs are possible, this
patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
52-bit value.

Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Steve Capper <steve.capper@arm.com>

---

Changed in V5, removed ttbr1 save/restore logic for software PAN as
hardware PAN is a mandatory ARMv8.1 feature anyway. The logic to enable
52-bit VAs has also been changed to depend on
ARM64_PAN || !ARM64_SW_TTBR0_PAN
(in a later patch)

This patch is new in V4 of the series
---
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/pgtable-hwdef.h |  9 +++++++++
 arch/arm64/kernel/head.S               |  1 +
 arch/arm64/kernel/hibernate-asm.S      |  1 +
 arch/arm64/mm/proc.S                   |  4 ++++
 5 files changed, 38 insertions(+)

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 6142402c2eb4..e2fe378d2a63 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -515,6 +515,29 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
 	mrs	\rd, sp_el0
 	.endm
 
+/*
+ * Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD.
+ * orr is used as it can cover the immediate value (and is idempotent).
+ * In future this may be nop'ed out when dealing with 52-bit kernel VAs.
+ * 	ttbr: Value of ttbr to set, modified.
+ */
+	.macro	offset_ttbr1, ttbr
+#ifdef CONFIG_ARM64_52BIT_VA
+	orr	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
+#endif
+	.endm
+
+/*
+ * Perform the reverse of offset_ttbr1.
+ * bic is used as it can cover the immediate value and, in future, won't need
+ * to be nop'ed out when dealing with 52-bit kernel VAs.
+ */
+	.macro	restore_ttbr1, ttbr
+#ifdef CONFIG_ARM64_52BIT_VA
+	bic	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
+#endif
+	.endm
+
 /*
  * Arrange a physical address in a TTBR register, taking care of 52-bit
  * addresses.
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 1d7d8da2ef9b..4a29c7e03ae4 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -80,7 +80,11 @@
 #define PGDIR_SHIFT		ARM64_HW_PGTABLE_LEVEL_SHIFT(4 - CONFIG_PGTABLE_LEVELS)
 #define PGDIR_SIZE		(_AC(1, UL) << PGDIR_SHIFT)
 #define PGDIR_MASK		(~(PGDIR_SIZE-1))
+#ifdef CONFIG_ARM64_52BIT_VA
+#define PTRS_PER_PGD		(1 << (52 - PGDIR_SHIFT))
+#else
 #define PTRS_PER_PGD		(1 << (VA_BITS - PGDIR_SHIFT))
+#endif
 
 /*
  * Section address mask and size definitions.
@@ -306,4 +310,9 @@
 #define TTBR_BADDR_MASK_52	(((UL(1) << 46) - 1) << 2)
 #endif
 
+#ifdef CONFIG_ARM64_52BIT_VA
+#define TTBR1_BADDR_4852_OFFSET	(((UL(1) << (52 - PGDIR_SHIFT)) - \
+				 (UL(1) << (48 - PGDIR_SHIFT))) * 8)
+#endif
+
 #endif
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 4471f570a295..f60081be9a1b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -769,6 +769,7 @@ ENTRY(__enable_mmu)
 	phys_to_ttbr x1, x1
 	phys_to_ttbr x2, x2
 	msr	ttbr0_el1, x2			// load TTBR0
+	offset_ttbr1 x1
 	msr	ttbr1_el1, x1			// load TTBR1
 	isb
 	msr	sctlr_el1, x0
diff --git a/arch/arm64/kernel/hibernate-asm.S b/arch/arm64/kernel/hibernate-asm.S
index dd14ab8c9f72..fe36d85c60bd 100644
--- a/arch/arm64/kernel/hibernate-asm.S
+++ b/arch/arm64/kernel/hibernate-asm.S
@@ -40,6 +40,7 @@
 	tlbi	vmalle1
 	dsb	nsh
 	phys_to_ttbr \tmp, \page_table
+	offset_ttbr1 \tmp
 	msr	ttbr1_el1, \tmp
 	isb
 .endm
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 2c75b0b903ae..2db1c491d45d 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -182,6 +182,7 @@ ENDPROC(cpu_do_switch_mm)
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
 	phys_to_ttbr \tmp2, \tmp1
+	offset_ttbr1 \tmp2
 	msr	ttbr1_el1, \tmp2
 	isb
 	tlbi	vmalle1
@@ -200,6 +201,7 @@ ENTRY(idmap_cpu_replace_ttbr1)
 
 	__idmap_cpu_set_reserved_ttbr1 x1, x3
 
+	offset_ttbr1 x0
 	msr	ttbr1_el1, x0
 	isb
 
@@ -254,6 +256,7 @@ ENTRY(idmap_kpti_install_ng_mappings)
 	pte		.req	x16
 
 	mrs	swapper_ttb, ttbr1_el1
+	restore_ttbr1	swapper_ttb
 	adr	flag_ptr, __idmap_kpti_flag
 
 	cbnz	cpu, __idmap_kpti_secondary
@@ -373,6 +376,7 @@ __idmap_kpti_secondary:
 	cbnz	w18, 1b
 
 	/* All done, act like nothing happened */
+	offset_ttbr1 swapper_ttb
 	msr	ttbr1_el1, swapper_ttb
 	isb
 	ret
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
we have to be careful in allowing secondary CPUs to boot if 52-bit
virtual addresses have already been enabled on the boot CPU.

This patch adds code to the secondary startup path. If the boot CPU has
enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
secondary can also enable 52-bit support. If not, the secondary is
prevented from booting and an error message is displayed indicating why.

Technically this patch could be implemented using the cpufeature code
when considering 52-bit userspace support. However, we employ low level
checks here as the cpufeature code won't be able to run if we have
mismatched 52-bit kernel va support.

Signed-off-by: Steve Capper <steve.capper@arm.com>

---

Patch is new in V5 of the series
---
 arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
 arch/arm64/kernel/smp.c  |  5 +++++
 2 files changed, 31 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index f60081be9a1b..58fcc1edd852 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -707,6 +707,7 @@ secondary_startup:
 	/*
 	 * Common entry point for secondary CPUs.
 	 */
+	bl	__cpu_secondary_check52bitva
 	bl	__cpu_setup			// initialise processor
 	adrp	x1, swapper_pg_dir
 	bl	__enable_mmu
@@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
 	ret
 ENDPROC(__enable_mmu)
 
+ENTRY(__cpu_secondary_check52bitva)
+#ifdef CONFIG_ARM64_52BIT_VA
+	ldr_l	x0, vabits_user
+	cmp	x0, #52
+	b.ne	2f
+
+	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
+	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
+	cbnz	x0, 2f
+
+	adr_l	x0, va52mismatch
+	mov	w1, #1
+	strb	w1, [x0]
+	dmb	sy
+	dc	ivac, x0	// Invalidate potentially stale cache line
+
+	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x0, x1
+1:	wfe
+	wfi
+	b	1b
+
+#endif
+2:	ret
+ENDPROC(__cpu_secondary_check52bitva)
+
 __no_granule_support:
 	/* Indicate that this CPU can't boot and is stuck in the kernel */
 	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x1, x2
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 96b8f2f51ab2..e15b0b64d4d0 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -108,6 +108,7 @@ static int boot_secondary(unsigned int cpu, struct task_struct *idle)
 }
 
 static DECLARE_COMPLETION(cpu_running);
+bool va52mismatch __ro_after_init;
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)
 {
@@ -137,6 +138,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 
 		if (!cpu_online(cpu)) {
 			pr_crit("CPU%u: failed to come online\n", cpu);
+
+			if (IS_ENABLED(CONFIG_ARM64_52BIT_VA) && va52mismatch)
+				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
+
 			ret = -EIO;
 		}
 	} else {
-- 
2.19.2

[PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
we have to be careful in allowing secondary CPUs to boot if 52-bit
virtual addresses have already been enabled on the boot CPU.

This patch adds code to the secondary startup path. If the boot CPU has
enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
secondary can also enable 52-bit support. If not, the secondary is
prevented from booting and an error message is displayed indicating why.

Technically this patch could be implemented using the cpufeature code
when considering 52-bit userspace support. However, we employ low level
checks here as the cpufeature code won't be able to run if we have
mismatched 52-bit kernel va support.

Signed-off-by: Steve Capper <steve.capper@arm.com>

---

Patch is new in V5 of the series
---
 arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
 arch/arm64/kernel/smp.c  |  5 +++++
 2 files changed, 31 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index f60081be9a1b..58fcc1edd852 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -707,6 +707,7 @@ secondary_startup:
 	/*
 	 * Common entry point for secondary CPUs.
 	 */
+	bl	__cpu_secondary_check52bitva
 	bl	__cpu_setup			// initialise processor
 	adrp	x1, swapper_pg_dir
 	bl	__enable_mmu
@@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
 	ret
 ENDPROC(__enable_mmu)
 
+ENTRY(__cpu_secondary_check52bitva)
+#ifdef CONFIG_ARM64_52BIT_VA
+	ldr_l	x0, vabits_user
+	cmp	x0, #52
+	b.ne	2f
+
+	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
+	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
+	cbnz	x0, 2f
+
+	adr_l	x0, va52mismatch
+	mov	w1, #1
+	strb	w1, [x0]
+	dmb	sy
+	dc	ivac, x0	// Invalidate potentially stale cache line
+
+	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x0, x1
+1:	wfe
+	wfi
+	b	1b
+
+#endif
+2:	ret
+ENDPROC(__cpu_secondary_check52bitva)
+
 __no_granule_support:
 	/* Indicate that this CPU can't boot and is stuck in the kernel */
 	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x1, x2
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 96b8f2f51ab2..e15b0b64d4d0 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -108,6 +108,7 @@ static int boot_secondary(unsigned int cpu, struct task_struct *idle)
 }
 
 static DECLARE_COMPLETION(cpu_running);
+bool va52mismatch __ro_after_init;
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)
 {
@@ -137,6 +138,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 
 		if (!cpu_online(cpu)) {
 			pr_crit("CPU%u: failed to come online\n", cpu);
+
+			if (IS_ENABLED(CONFIG_ARM64_52BIT_VA) && va52mismatch)
+				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
+
 			ret = -EIO;
 		}
 	} else {
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 6/7] arm64: mm: introduce 52-bit userspace support

[Steve Capper]

On arm64 there is optional support for a 52-bit virtual address space.
To exploit this one has to be running with a 64KB page size and be
running on hardware that supports this.

For an arm64 kernel supporting a 48 bit VA with a 64KB page size,
some changes are needed to support a 52-bit userspace:
 * TCR_EL1.T0SZ needs to be 12 instead of 16,
 * TASK_SIZE needs to reflect the new size.

This patch implements the above when the support for 52-bit VAs is
detected at early boot time.

On arm64 userspace addresses translation is controlled by TTBR0_EL1. As
well as userspace, TTBR0_EL1 controls:
 * The identity mapping,
 * EFI runtime code.

It is possible to run a kernel with an identity mapping that has a
larger VA size than userspace (and for this case __cpu_set_tcr_t0sz()
would set TCR_EL1.T0SZ as appropriate). However, when the conditions for
52-bit userspace are met; it is possible to keep TCR_EL1.T0SZ fixed at
12. Thus in this patch, the TCR_EL1.T0SZ size changing logic is
disabled.

Signed-off-by: Steve Capper <steve.capper@arm.com>

---
Changed in V5, extra dependency for 52-bit support:
ARM64_PAN || !ARM64_SW_TTBR0_PAN

Changed in V4, pgd_index logic removed as we offset ttbr1 instead
---
 arch/arm64/Kconfig                   |  4 ++++
 arch/arm64/include/asm/assembler.h   |  7 +++----
 arch/arm64/include/asm/mmu_context.h |  3 +++
 arch/arm64/include/asm/processor.h   | 14 +++++++++-----
 arch/arm64/kernel/head.S             | 13 +++++++++++++
 arch/arm64/mm/fault.c                |  2 +-
 arch/arm64/mm/mmu.c                  |  1 +
 arch/arm64/mm/proc.S                 | 10 +++++++++-
 8 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 787d7850e064..6a93d5bc7f76 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -709,6 +709,10 @@ config ARM64_PA_BITS_52
 
 endchoice
 
+config ARM64_52BIT_VA
+	def_bool y
+	depends on ARM64_VA_BITS_48 && ARM64_64K_PAGES && (ARM64_PAN || !ARM64_SW_TTBR0_PAN)
+
 config ARM64_PA_BITS
 	int
 	default 48 if ARM64_PA_BITS_48
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index e2fe378d2a63..243ec4f0c00f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -342,11 +342,10 @@ alternative_endif
 	.endm
 
 /*
- * tcr_set_idmap_t0sz - update TCR.T0SZ so that we can load the ID map
+ * tcr_set_t0sz - update TCR.T0SZ so that we can load the ID map
  */
-	.macro	tcr_set_idmap_t0sz, valreg, tmpreg
-	ldr_l	\tmpreg, idmap_t0sz
-	bfi	\valreg, \tmpreg, #TCR_T0SZ_OFFSET, #TCR_TxSZ_WIDTH
+	.macro	tcr_set_t0sz, valreg, t0sz
+	bfi	\valreg, \t0sz, #TCR_T0SZ_OFFSET, #TCR_TxSZ_WIDTH
 	.endm
 
 /*
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 1e58bf58c22b..b125fafc611b 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -72,6 +72,9 @@ extern u64 idmap_ptrs_per_pgd;
 
 static inline bool __cpu_uses_extended_idmap(void)
 {
+	if (IS_ENABLED(CONFIG_ARM64_52BIT_VA))
+		return false;
+
 	return unlikely(idmap_t0sz != TCR_T0SZ(VA_BITS));
 }
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fe95fd8b065e..b363fc705be4 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -19,11 +19,12 @@
 #ifndef __ASM_PROCESSOR_H
 #define __ASM_PROCESSOR_H
 
-#define TASK_SIZE_64		(UL(1) << VA_BITS)
-
-#define KERNEL_DS	UL(-1)
-#define USER_DS		(TASK_SIZE_64 - 1)
-
+#define KERNEL_DS		UL(-1)
+#ifdef CONFIG_ARM64_52BIT_VA
+#define USER_DS			((UL(1) << 52) - 1)
+#else
+#define USER_DS			((UL(1) << VA_BITS) - 1)
+#endif /* CONFIG_ARM64_52IT_VA */
 #ifndef __ASSEMBLY__
 #ifdef __KERNEL__
 
@@ -48,6 +49,9 @@
 
 #define DEFAULT_MAP_WINDOW_64	(UL(1) << VA_BITS)
 
+extern u64 vabits_user;
+#define TASK_SIZE_64		(UL(1) << vabits_user)
+
 #ifdef CONFIG_COMPAT
 #define TASK_SIZE_32		UL(0x100000000)
 #define TASK_SIZE		(test_thread_flag(TIF_32BIT) ? \
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 58fcc1edd852..c229d9cfe9bf 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -318,6 +318,19 @@ __create_page_tables:
 	adrp	x0, idmap_pg_dir
 	adrp	x3, __idmap_text_start		// __pa(__idmap_text_start)
 
+#ifdef CONFIG_ARM64_52BIT_VA
+	mrs_s	x6, SYS_ID_AA64MMFR2_EL1
+	and	x6, x6, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
+	mov	x5, #52
+	cbnz	x6, 1f
+#endif
+	mov	x5, #VA_BITS
+1:
+	adr_l	x6, vabits_user
+	str	x5, [x6]
+	dmb	sy
+	dc	ivac, x6		// Invalidate potentially stale cache line
+
 	/*
 	 * VA_BITS may be too small to allow for an ID mapping to be created
 	 * that covers system RAM if that is located sufficiently high in the
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 7d9571f4ae3d..5fe6d2e40e9b 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -160,7 +160,7 @@ void show_pte(unsigned long addr)
 
 	pr_alert("%s pgtable: %luk pages, %u-bit VAs, pgdp = %p\n",
 		 mm == &init_mm ? "swapper" : "user", PAGE_SIZE / SZ_1K,
-		 VA_BITS, mm->pgd);
+		 mm == &init_mm ? VA_BITS : (int) vabits_user, mm->pgd);
 	pgdp = pgd_offset(mm, addr);
 	pgd = READ_ONCE(*pgdp);
 	pr_alert("[%016lx] pgd=%016llx", addr, pgd_val(pgd));
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 394b8d554def..f8fc393143ea 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -52,6 +52,7 @@
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
 u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
+u64 vabits_user __ro_after_init;
 
 u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 2db1c491d45d..0cf86b17714c 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -450,7 +450,15 @@ ENTRY(__cpu_setup)
 	ldr	x10, =TCR_TxSZ(VA_BITS) | TCR_CACHE_FLAGS | TCR_SMP_FLAGS | \
 			TCR_TG_FLAGS | TCR_KASLR_FLAGS | TCR_ASID16 | \
 			TCR_TBI0 | TCR_A1
-	tcr_set_idmap_t0sz	x10, x9
+
+#ifdef CONFIG_ARM64_52BIT_VA
+	ldr_l 		x9, vabits_user
+	sub		x9, xzr, x9
+	add		x9, x9, #64
+#else
+	ldr_l		x9, idmap_t0sz
+#endif
+	tcr_set_t0sz	x10, x9
 
 	/*
 	 * Set the IPS bits in TCR_EL1.
-- 
2.19.2

[PATCH V5 6/7] arm64: mm: introduce 52-bit userspace support

[Steve Capper]

On arm64 there is optional support for a 52-bit virtual address space.
To exploit this one has to be running with a 64KB page size and be
running on hardware that supports this.

For an arm64 kernel supporting a 48 bit VA with a 64KB page size,
some changes are needed to support a 52-bit userspace:
 * TCR_EL1.T0SZ needs to be 12 instead of 16,
 * TASK_SIZE needs to reflect the new size.

This patch implements the above when the support for 52-bit VAs is
detected at early boot time.

On arm64 userspace addresses translation is controlled by TTBR0_EL1. As
well as userspace, TTBR0_EL1 controls:
 * The identity mapping,
 * EFI runtime code.

It is possible to run a kernel with an identity mapping that has a
larger VA size than userspace (and for this case __cpu_set_tcr_t0sz()
would set TCR_EL1.T0SZ as appropriate). However, when the conditions for
52-bit userspace are met; it is possible to keep TCR_EL1.T0SZ fixed at
12. Thus in this patch, the TCR_EL1.T0SZ size changing logic is
disabled.

Signed-off-by: Steve Capper <steve.capper@arm.com>

---
Changed in V5, extra dependency for 52-bit support:
ARM64_PAN || !ARM64_SW_TTBR0_PAN

Changed in V4, pgd_index logic removed as we offset ttbr1 instead
---
 arch/arm64/Kconfig                   |  4 ++++
 arch/arm64/include/asm/assembler.h   |  7 +++----
 arch/arm64/include/asm/mmu_context.h |  3 +++
 arch/arm64/include/asm/processor.h   | 14 +++++++++-----
 arch/arm64/kernel/head.S             | 13 +++++++++++++
 arch/arm64/mm/fault.c                |  2 +-
 arch/arm64/mm/mmu.c                  |  1 +
 arch/arm64/mm/proc.S                 | 10 +++++++++-
 8 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 787d7850e064..6a93d5bc7f76 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -709,6 +709,10 @@ config ARM64_PA_BITS_52
 
 endchoice
 
+config ARM64_52BIT_VA
+	def_bool y
+	depends on ARM64_VA_BITS_48 && ARM64_64K_PAGES && (ARM64_PAN || !ARM64_SW_TTBR0_PAN)
+
 config ARM64_PA_BITS
 	int
 	default 48 if ARM64_PA_BITS_48
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index e2fe378d2a63..243ec4f0c00f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -342,11 +342,10 @@ alternative_endif
 	.endm
 
 /*
- * tcr_set_idmap_t0sz - update TCR.T0SZ so that we can load the ID map
+ * tcr_set_t0sz - update TCR.T0SZ so that we can load the ID map
  */
-	.macro	tcr_set_idmap_t0sz, valreg, tmpreg
-	ldr_l	\tmpreg, idmap_t0sz
-	bfi	\valreg, \tmpreg, #TCR_T0SZ_OFFSET, #TCR_TxSZ_WIDTH
+	.macro	tcr_set_t0sz, valreg, t0sz
+	bfi	\valreg, \t0sz, #TCR_T0SZ_OFFSET, #TCR_TxSZ_WIDTH
 	.endm
 
 /*
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 1e58bf58c22b..b125fafc611b 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -72,6 +72,9 @@ extern u64 idmap_ptrs_per_pgd;
 
 static inline bool __cpu_uses_extended_idmap(void)
 {
+	if (IS_ENABLED(CONFIG_ARM64_52BIT_VA))
+		return false;
+
 	return unlikely(idmap_t0sz != TCR_T0SZ(VA_BITS));
 }
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fe95fd8b065e..b363fc705be4 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -19,11 +19,12 @@
 #ifndef __ASM_PROCESSOR_H
 #define __ASM_PROCESSOR_H
 
-#define TASK_SIZE_64		(UL(1) << VA_BITS)
-
-#define KERNEL_DS	UL(-1)
-#define USER_DS		(TASK_SIZE_64 - 1)
-
+#define KERNEL_DS		UL(-1)
+#ifdef CONFIG_ARM64_52BIT_VA
+#define USER_DS			((UL(1) << 52) - 1)
+#else
+#define USER_DS			((UL(1) << VA_BITS) - 1)
+#endif /* CONFIG_ARM64_52IT_VA */
 #ifndef __ASSEMBLY__
 #ifdef __KERNEL__
 
@@ -48,6 +49,9 @@
 
 #define DEFAULT_MAP_WINDOW_64	(UL(1) << VA_BITS)
 
+extern u64 vabits_user;
+#define TASK_SIZE_64		(UL(1) << vabits_user)
+
 #ifdef CONFIG_COMPAT
 #define TASK_SIZE_32		UL(0x100000000)
 #define TASK_SIZE		(test_thread_flag(TIF_32BIT) ? \
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 58fcc1edd852..c229d9cfe9bf 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -318,6 +318,19 @@ __create_page_tables:
 	adrp	x0, idmap_pg_dir
 	adrp	x3, __idmap_text_start		// __pa(__idmap_text_start)
 
+#ifdef CONFIG_ARM64_52BIT_VA
+	mrs_s	x6, SYS_ID_AA64MMFR2_EL1
+	and	x6, x6, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
+	mov	x5, #52
+	cbnz	x6, 1f
+#endif
+	mov	x5, #VA_BITS
+1:
+	adr_l	x6, vabits_user
+	str	x5, [x6]
+	dmb	sy
+	dc	ivac, x6		// Invalidate potentially stale cache line
+
 	/*
 	 * VA_BITS may be too small to allow for an ID mapping to be created
 	 * that covers system RAM if that is located sufficiently high in the
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 7d9571f4ae3d..5fe6d2e40e9b 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -160,7 +160,7 @@ void show_pte(unsigned long addr)
 
 	pr_alert("%s pgtable: %luk pages, %u-bit VAs, pgdp = %p\n",
 		 mm == &init_mm ? "swapper" : "user", PAGE_SIZE / SZ_1K,
-		 VA_BITS, mm->pgd);
+		 mm == &init_mm ? VA_BITS : (int) vabits_user, mm->pgd);
 	pgdp = pgd_offset(mm, addr);
 	pgd = READ_ONCE(*pgdp);
 	pr_alert("[%016lx] pgd=%016llx", addr, pgd_val(pgd));
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 394b8d554def..f8fc393143ea 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -52,6 +52,7 @@
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
 u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
+u64 vabits_user __ro_after_init;
 
 u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 2db1c491d45d..0cf86b17714c 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -450,7 +450,15 @@ ENTRY(__cpu_setup)
 	ldr	x10, =TCR_TxSZ(VA_BITS) | TCR_CACHE_FLAGS | TCR_SMP_FLAGS | \
 			TCR_TG_FLAGS | TCR_KASLR_FLAGS | TCR_ASID16 | \
 			TCR_TBI0 | TCR_A1
-	tcr_set_idmap_t0sz	x10, x9
+
+#ifdef CONFIG_ARM64_52BIT_VA
+	ldr_l 		x9, vabits_user
+	sub		x9, xzr, x9
+	add		x9, x9, #64
+#else
+	ldr_l		x9, idmap_t0sz
+#endif
+	tcr_set_t0sz	x10, x9
 
 	/*
 	 * Set the IPS bits in TCR_EL1.
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 7/7] arm64: mm: Allow forcing all userspace addresses to 52-bit

[Steve Capper]

On arm64 52-bit VAs are provided to userspace when a hint is supplied to
mmap. This helps maintain compatibility with software that expects at
most 48-bit VAs to be returned.

In order to help identify software that has 48-bit VA assumptions, this
patch allows one to compile a kernel where 52-bit VAs are returned by
default on HW that supports it.

This feature is intended to be for development systems only.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig                 | 13 +++++++++++++
 arch/arm64/include/asm/elf.h       |  4 ++++
 arch/arm64/include/asm/processor.h |  9 ++++++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6a93d5bc7f76..12658f05bb41 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1165,6 +1165,19 @@ config ARM64_CNP
 	  at runtime, and does not affect PEs that do not implement
 	  this feature.
 
+config ARM64_FORCE_52BIT
+	bool "Force 52-bit virtual addresses for userspace"
+	depends on ARM64_52BIT_VA && EXPERT
+	help
+	  For systems with 52-bit userspace VAs enabled, the kernel will attempt
+	  to maintain compatibility with older software by providing 48-bit VAs
+	  unless a hint is supplied to mmap.
+
+	  This configuration option disables the 48-bit compatibility logic, and
+	  forces all userspace addresses to be 52-bit on HW that supports it. One
+	  should only enable this configuration option for stress testing userspace
+	  memory management code. If unsure say N here.
+
 endmenu
 
 config ARM64_SVE
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index bc9bd9e77d9d..6adc1a90e7e6 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -117,7 +117,11 @@
  * 64-bit, this is above 4GB to leave the entire 32-bit address
  * space open for things that want to use the area for 32-bit pointers.
  */
+#ifdef CONFIG_ARM64_FORCE_52BIT
+#define ELF_ET_DYN_BASE		(2 * TASK_SIZE_64 / 3)
+#else
 #define ELF_ET_DYN_BASE		(2 * DEFAULT_MAP_WINDOW_64 / 3)
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index b363fc705be4..9abd91570b5b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -65,8 +65,13 @@ extern u64 vabits_user;
 #define DEFAULT_MAP_WINDOW	DEFAULT_MAP_WINDOW_64
 #endif /* CONFIG_COMPAT */
 
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#ifdef CONFIG_ARM64_FORCE_52BIT
+#define STACK_TOP_MAX		TASK_SIZE_64
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 4))
+#else
 #define STACK_TOP_MAX		DEFAULT_MAP_WINDOW_64
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 #ifdef CONFIG_COMPAT
 #define AARCH32_VECTORS_BASE	0xffff0000
@@ -76,12 +81,14 @@ extern u64 vabits_user;
 #define STACK_TOP		STACK_TOP_MAX
 #endif /* CONFIG_COMPAT */
 
+#ifndef CONFIG_ARM64_FORCE_52BIT
 #define arch_get_mmap_end(addr) ((addr > DEFAULT_MAP_WINDOW) ? TASK_SIZE :\
 				DEFAULT_MAP_WINDOW)
 
 #define arch_get_mmap_base(addr, base) ((addr > DEFAULT_MAP_WINDOW) ? \
 					base + TASK_SIZE - DEFAULT_MAP_WINDOW :\
 					base)
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT	(arm64_dma_phys_limit - 1)
-- 
2.19.2

[PATCH V5 7/7] arm64: mm: Allow forcing all userspace addresses to 52-bit

[Steve Capper]

On arm64 52-bit VAs are provided to userspace when a hint is supplied to
mmap. This helps maintain compatibility with software that expects at
most 48-bit VAs to be returned.

In order to help identify software that has 48-bit VA assumptions, this
patch allows one to compile a kernel where 52-bit VAs are returned by
default on HW that supports it.

This feature is intended to be for development systems only.

Signed-off-by: Steve Capper <steve.capper@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig                 | 13 +++++++++++++
 arch/arm64/include/asm/elf.h       |  4 ++++
 arch/arm64/include/asm/processor.h |  9 ++++++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6a93d5bc7f76..12658f05bb41 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1165,6 +1165,19 @@ config ARM64_CNP
 	  at runtime, and does not affect PEs that do not implement
 	  this feature.
 
+config ARM64_FORCE_52BIT
+	bool "Force 52-bit virtual addresses for userspace"
+	depends on ARM64_52BIT_VA && EXPERT
+	help
+	  For systems with 52-bit userspace VAs enabled, the kernel will attempt
+	  to maintain compatibility with older software by providing 48-bit VAs
+	  unless a hint is supplied to mmap.
+
+	  This configuration option disables the 48-bit compatibility logic, and
+	  forces all userspace addresses to be 52-bit on HW that supports it. One
+	  should only enable this configuration option for stress testing userspace
+	  memory management code. If unsure say N here.
+
 endmenu
 
 config ARM64_SVE
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index bc9bd9e77d9d..6adc1a90e7e6 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -117,7 +117,11 @@
  * 64-bit, this is above 4GB to leave the entire 32-bit address
  * space open for things that want to use the area for 32-bit pointers.
  */
+#ifdef CONFIG_ARM64_FORCE_52BIT
+#define ELF_ET_DYN_BASE		(2 * TASK_SIZE_64 / 3)
+#else
 #define ELF_ET_DYN_BASE		(2 * DEFAULT_MAP_WINDOW_64 / 3)
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index b363fc705be4..9abd91570b5b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -65,8 +65,13 @@ extern u64 vabits_user;
 #define DEFAULT_MAP_WINDOW	DEFAULT_MAP_WINDOW_64
 #endif /* CONFIG_COMPAT */
 
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#ifdef CONFIG_ARM64_FORCE_52BIT
+#define STACK_TOP_MAX		TASK_SIZE_64
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 4))
+#else
 #define STACK_TOP_MAX		DEFAULT_MAP_WINDOW_64
+#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4))
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 #ifdef CONFIG_COMPAT
 #define AARCH32_VECTORS_BASE	0xffff0000
@@ -76,12 +81,14 @@ extern u64 vabits_user;
 #define STACK_TOP		STACK_TOP_MAX
 #endif /* CONFIG_COMPAT */
 
+#ifndef CONFIG_ARM64_FORCE_52BIT
 #define arch_get_mmap_end(addr) ((addr > DEFAULT_MAP_WINDOW) ? TASK_SIZE :\
 				DEFAULT_MAP_WINDOW)
 
 #define arch_get_mmap_base(addr, base) ((addr > DEFAULT_MAP_WINDOW) ? \
 					base + TASK_SIZE - DEFAULT_MAP_WINDOW :\
 					base)
+#endif /* CONFIG_ARM64_FORCE_52BIT */
 
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT	(arm64_dma_phys_limit - 1)
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

Hi Steve,

On 12/06/2018 10:50 PM, Steve Capper wrote:
> For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
> we have to be careful in allowing secondary CPUs to boot if 52-bit
> virtual addresses have already been enabled on the boot CPU.
> 
> This patch adds code to the secondary startup path. If the boot CPU has
> enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
> secondary can also enable 52-bit support. If not, the secondary is
> prevented from booting and an error message is displayed indicating why.
> 
> Technically this patch could be implemented using the cpufeature code
> when considering 52-bit userspace support. However, we employ low level
> checks here as the cpufeature code won't be able to run if we have
> mismatched 52-bit kernel va support.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> 

The patch looks good to me, except for one comment below.

> ---
> 
> Patch is new in V5 of the series
> ---
>   arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
>   arch/arm64/kernel/smp.c  |  5 +++++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index f60081be9a1b..58fcc1edd852 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -707,6 +707,7 @@ secondary_startup:
>   	/*
>   	 * Common entry point for secondary CPUs.
>   	 */
> +	bl	__cpu_secondary_check52bitva
>   	bl	__cpu_setup			// initialise processor
>   	adrp	x1, swapper_pg_dir
>   	bl	__enable_mmu
> @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
>   	ret
>   ENDPROC(__enable_mmu)
>   
> +ENTRY(__cpu_secondary_check52bitva)
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	ldr_l	x0, vabits_user
> +	cmp	x0, #52
> +	b.ne	2f > +
> +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> +	cbnz	x0, 2f
> +
> +	adr_l	x0, va52mismatch
> +	mov	w1, #1
> +	strb	w1, [x0]
> +	dmb	sy
> +	dc	ivac, x0	// Invalidate potentially stale cache line

You may have to clear this variable before a CPU is brought up to avoid 
raising a false error message when another secondary CPU doesn't boot
for some other reason (say granule support) after a CPU failed with lack
of 52bitva. It is really a crazy corner case.

Otherwise looks good to me.

Cheers
Suzuki

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

Hi Steve,

On 12/06/2018 10:50 PM, Steve Capper wrote:
> For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
> we have to be careful in allowing secondary CPUs to boot if 52-bit
> virtual addresses have already been enabled on the boot CPU.
> 
> This patch adds code to the secondary startup path. If the boot CPU has
> enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
> secondary can also enable 52-bit support. If not, the secondary is
> prevented from booting and an error message is displayed indicating why.
> 
> Technically this patch could be implemented using the cpufeature code
> when considering 52-bit userspace support. However, we employ low level
> checks here as the cpufeature code won't be able to run if we have
> mismatched 52-bit kernel va support.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> 

The patch looks good to me, except for one comment below.

> ---
> 
> Patch is new in V5 of the series
> ---
>   arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
>   arch/arm64/kernel/smp.c  |  5 +++++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index f60081be9a1b..58fcc1edd852 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -707,6 +707,7 @@ secondary_startup:
>   	/*
>   	 * Common entry point for secondary CPUs.
>   	 */
> +	bl	__cpu_secondary_check52bitva
>   	bl	__cpu_setup			// initialise processor
>   	adrp	x1, swapper_pg_dir
>   	bl	__enable_mmu
> @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
>   	ret
>   ENDPROC(__enable_mmu)
>   
> +ENTRY(__cpu_secondary_check52bitva)
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	ldr_l	x0, vabits_user
> +	cmp	x0, #52
> +	b.ne	2f > +
> +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> +	cbnz	x0, 2f
> +
> +	adr_l	x0, va52mismatch
> +	mov	w1, #1
> +	strb	w1, [x0]
> +	dmb	sy
> +	dc	ivac, x0	// Invalidate potentially stale cache line

You may have to clear this variable before a CPU is brought up to avoid 
raising a false error message when another secondary CPU doesn't boot
for some other reason (say granule support) after a CPU failed with lack
of 52bitva. It is really a crazy corner case.

Otherwise looks good to me.

Cheers
Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Catalin Marinas]

On Thu, Dec 06, 2018 at 10:50:39PM +0000, Steve Capper wrote:
> Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
> entries (for the 48-bit case) to 1024 entries. This quantity,
> PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
> to a given virtual address, addr:
> 
> pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)
> 
> Userspace addresses are prefixed by 0's, so for a 48-bit userspace
> address, uva, the following is true:
> (uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words, a 48-bit userspace address will have the same pgd_index
> when using PTRS_PER_PGD = 64 and 1024.
> 
> Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
> kva, we have the following inequality:
> (kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words a 48-bit kernel virtual address will have a different
> pgd_index when using PTRS_PER_PGD = 64 and 1024.
> 
> If, however, we note that:
> kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
> and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)
> 
> We can consider:
> (kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
>  = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
>  = 0x3C0
> 
> In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
> provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
> running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).
> 
> For kernel configuration where 52-bit userspace VAs are possible, this
> patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
> 52-bit value.
> 
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Steve Capper <steve.capper@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Catalin Marinas]

On Thu, Dec 06, 2018 at 10:50:39PM +0000, Steve Capper wrote:
> Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
> entries (for the 48-bit case) to 1024 entries. This quantity,
> PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
> to a given virtual address, addr:
> 
> pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)
> 
> Userspace addresses are prefixed by 0's, so for a 48-bit userspace
> address, uva, the following is true:
> (uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words, a 48-bit userspace address will have the same pgd_index
> when using PTRS_PER_PGD = 64 and 1024.
> 
> Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
> kva, we have the following inequality:
> (kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words a 48-bit kernel virtual address will have a different
> pgd_index when using PTRS_PER_PGD = 64 and 1024.
> 
> If, however, we note that:
> kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
> and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)
> 
> We can consider:
> (kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
>  = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
>  = 0x3C0
> 
> In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
> provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
> running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).
> 
> For kernel configuration where 52-bit userspace VAs are possible, this
> patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
> 52-bit value.
> 
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Steve Capper <steve.capper@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 6/7] arm64: mm: introduce 52-bit userspace support

[Catalin Marinas]

On Thu, Dec 06, 2018 at 10:50:41PM +0000, Steve Capper wrote:
> On arm64 there is optional support for a 52-bit virtual address space.
> To exploit this one has to be running with a 64KB page size and be
> running on hardware that supports this.
> 
> For an arm64 kernel supporting a 48 bit VA with a 64KB page size,
> some changes are needed to support a 52-bit userspace:
>  * TCR_EL1.T0SZ needs to be 12 instead of 16,
>  * TASK_SIZE needs to reflect the new size.
> 
> This patch implements the above when the support for 52-bit VAs is
> detected at early boot time.
> 
> On arm64 userspace addresses translation is controlled by TTBR0_EL1. As
> well as userspace, TTBR0_EL1 controls:
>  * The identity mapping,
>  * EFI runtime code.
> 
> It is possible to run a kernel with an identity mapping that has a
> larger VA size than userspace (and for this case __cpu_set_tcr_t0sz()
> would set TCR_EL1.T0SZ as appropriate). However, when the conditions for
> 52-bit userspace are met; it is possible to keep TCR_EL1.T0SZ fixed at
> 12. Thus in this patch, the TCR_EL1.T0SZ size changing logic is
> disabled.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH V5 6/7] arm64: mm: introduce 52-bit userspace support

[Catalin Marinas]

On Thu, Dec 06, 2018 at 10:50:41PM +0000, Steve Capper wrote:
> On arm64 there is optional support for a 52-bit virtual address space.
> To exploit this one has to be running with a 64KB page size and be
> running on hardware that supports this.
> 
> For an arm64 kernel supporting a 48 bit VA with a 64KB page size,
> some changes are needed to support a 52-bit userspace:
>  * TCR_EL1.T0SZ needs to be 12 instead of 16,
>  * TASK_SIZE needs to reflect the new size.
> 
> This patch implements the above when the support for 52-bit VAs is
> detected at early boot time.
> 
> On arm64 userspace addresses translation is controlled by TTBR0_EL1. As
> well as userspace, TTBR0_EL1 controls:
>  * The identity mapping,
>  * EFI runtime code.
> 
> It is possible to run a kernel with an identity mapping that has a
> larger VA size than userspace (and for this case __cpu_set_tcr_t0sz()
> would set TCR_EL1.T0SZ as appropriate). However, when the conditions for
> 52-bit userspace are met; it is possible to keep TCR_EL1.T0SZ fixed at
> 12. Thus in this patch, the TCR_EL1.T0SZ size changing logic is
> disabled.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Suzuki K Poulose]

On 12/06/2018 10:50 PM, Steve Capper wrote:
> Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
> entries (for the 48-bit case) to 1024 entries. This quantity,
> PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
> to a given virtual address, addr:
> 
> pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)
> 
> Userspace addresses are prefixed by 0's, so for a 48-bit userspace
> address, uva, the following is true:
> (uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words, a 48-bit userspace address will have the same pgd_index
> when using PTRS_PER_PGD = 64 and 1024.
> 
> Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
> kva, we have the following inequality:
> (kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words a 48-bit kernel virtual address will have a different
> pgd_index when using PTRS_PER_PGD = 64 and 1024.
> 
> If, however, we note that:
> kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
> and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)
> 
> We can consider:
> (kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
>   = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
>   = 0x3C0
> 
> In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
> provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
> running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).
> 
> For kernel configuration where 52-bit userspace VAs are possible, this
> patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
> 52-bit value.
> 
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> 
> ---
> 
> Changed in V5, removed ttbr1 save/restore logic for software PAN as
> hardware PAN is a mandatory ARMv8.1 feature anyway. The logic to enable
> 52-bit VAs has also been changed to depend on
> ARM64_PAN || !ARM64_SW_TTBR0_PAN
> (in a later patch)
> 
> This patch is new in V4 of the series
> ---
>   arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
>   arch/arm64/include/asm/pgtable-hwdef.h |  9 +++++++++
>   arch/arm64/kernel/head.S               |  1 +
>   arch/arm64/kernel/hibernate-asm.S      |  1 +
>   arch/arm64/mm/proc.S                   |  4 ++++
>   5 files changed, 38 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 6142402c2eb4..e2fe378d2a63 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -515,6 +515,29 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
>   	mrs	\rd, sp_el0
>   	.endm
>   
> +/*
> + * Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD.
> + * orr is used as it can cover the immediate value (and is idempotent).
> + * In future this may be nop'ed out when dealing with 52-bit kernel VAs.
> + * 	ttbr: Value of ttbr to set, modified.
> + */
> +	.macro	offset_ttbr1, ttbr
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	orr	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
> +#endif
> +	.endm
> +
> +/*
> + * Perform the reverse of offset_ttbr1.
> + * bic is used as it can cover the immediate value and, in future, won't need
> + * to be nop'ed out when dealing with 52-bit kernel VAs.
> + */
> +	.macro	restore_ttbr1, ttbr
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	bic	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
> +#endif
> +	.endm
> +

The above operation is safe as long as the TTBR1_BADDR_4852_OFFSET is
aligned to 2^6 or more. Otherwise we could corrupt the Bits[51:48]
of the BADDR stored in TTBR1[5:2] and thus the TTBR1:BADDR must be 
aligned to 64bytes minimum as per v8.2LVA restrictions. Since we have
restricted the VA_BITS to 48, we should be safe here.


Do we need a BUILD_BUG_ON() or something to check if this is still valid?

Eitherway,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH V5 4/7] arm64: mm: Offset TTBR1 to allow 52-bit PTRS_PER_PGD

[Suzuki K Poulose]

On 12/06/2018 10:50 PM, Steve Capper wrote:
> Enabling 52-bit VAs on arm64 requires that the PGD table expands from 64
> entries (for the 48-bit case) to 1024 entries. This quantity,
> PTRS_PER_PGD is used as follows to compute which PGD entry corresponds
> to a given virtual address, addr:
> 
> pgd_index(addr) -> (addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)
> 
> Userspace addresses are prefixed by 0's, so for a 48-bit userspace
> address, uva, the following is true:
> (uva >> PGDIR_SHIFT) & (1024 - 1) == (uva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words, a 48-bit userspace address will have the same pgd_index
> when using PTRS_PER_PGD = 64 and 1024.
> 
> Kernel addresses are prefixed by 1's so, given a 48-bit kernel address,
> kva, we have the following inequality:
> (kva >> PGDIR_SHIFT) & (1024 - 1) != (kva >> PGDIR_SHIFT) & (64 - 1)
> 
> In other words a 48-bit kernel virtual address will have a different
> pgd_index when using PTRS_PER_PGD = 64 and 1024.
> 
> If, however, we note that:
> kva = 0xFFFF << 48 + lower (where lower[63:48] == 0b)
> and, PGDIR_SHIFT = 42 (as we are dealing with 64KB PAGE_SIZE)
> 
> We can consider:
> (kva >> PGDIR_SHIFT) & (1024 - 1) - (kva >> PGDIR_SHIFT) & (64 - 1)
>   = (0xFFFF << 6) & 0x3FF - (0xFFFF << 6) & 0x3F	// "lower" cancels out
>   = 0x3C0
> 
> In other words, one can switch PTRS_PER_PGD to the 52-bit value globally
> provided that they increment ttbr1_el1 by 0x3C0 * 8 = 0x1E00 bytes when
> running with 48-bit kernel VAs (TCR_EL1.T1SZ = 16).
> 
> For kernel configuration where 52-bit userspace VAs are possible, this
> patch offsets ttbr1_el1 and sets PTRS_PER_PGD corresponding to the
> 52-bit value.
> 
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> 
> ---
> 
> Changed in V5, removed ttbr1 save/restore logic for software PAN as
> hardware PAN is a mandatory ARMv8.1 feature anyway. The logic to enable
> 52-bit VAs has also been changed to depend on
> ARM64_PAN || !ARM64_SW_TTBR0_PAN
> (in a later patch)
> 
> This patch is new in V4 of the series
> ---
>   arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
>   arch/arm64/include/asm/pgtable-hwdef.h |  9 +++++++++
>   arch/arm64/kernel/head.S               |  1 +
>   arch/arm64/kernel/hibernate-asm.S      |  1 +
>   arch/arm64/mm/proc.S                   |  4 ++++
>   5 files changed, 38 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 6142402c2eb4..e2fe378d2a63 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -515,6 +515,29 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
>   	mrs	\rd, sp_el0
>   	.endm
>   
> +/*
> + * Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD.
> + * orr is used as it can cover the immediate value (and is idempotent).
> + * In future this may be nop'ed out when dealing with 52-bit kernel VAs.
> + * 	ttbr: Value of ttbr to set, modified.
> + */
> +	.macro	offset_ttbr1, ttbr
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	orr	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
> +#endif
> +	.endm
> +
> +/*
> + * Perform the reverse of offset_ttbr1.
> + * bic is used as it can cover the immediate value and, in future, won't need
> + * to be nop'ed out when dealing with 52-bit kernel VAs.
> + */
> +	.macro	restore_ttbr1, ttbr
> +#ifdef CONFIG_ARM64_52BIT_VA
> +	bic	\ttbr, \ttbr, #TTBR1_BADDR_4852_OFFSET
> +#endif
> +	.endm
> +

The above operation is safe as long as the TTBR1_BADDR_4852_OFFSET is
aligned to 2^6 or more. Otherwise we could corrupt the Bits[51:48]
of the BADDR stored in TTBR1[5:2] and thus the TTBR1:BADDR must be 
aligned to 64bytes minimum as per v8.2LVA restrictions. Since we have
restricted the VA_BITS to 48, we should be safe here.


Do we need a BUILD_BUG_ON() or something to check if this is still valid?

Eitherway,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> On 12/06/2018 10:50 PM, Steve Capper wrote:
> > For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
> > we have to be careful in allowing secondary CPUs to boot if 52-bit
> > virtual addresses have already been enabled on the boot CPU.
> > 
> > This patch adds code to the secondary startup path. If the boot CPU has
> > enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
> > secondary can also enable 52-bit support. If not, the secondary is
> > prevented from booting and an error message is displayed indicating why.
> > 
> > Technically this patch could be implemented using the cpufeature code
> > when considering 52-bit userspace support. However, we employ low level
> > checks here as the cpufeature code won't be able to run if we have
> > mismatched 52-bit kernel va support.
> > 
> > Signed-off-by: Steve Capper <steve.capper@arm.com>
> > 
> 
> The patch looks good to me, except for one comment below.
> 
> > ---
> > 
> > Patch is new in V5 of the series
> > ---
> >   arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
> >   arch/arm64/kernel/smp.c  |  5 +++++
> >   2 files changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index f60081be9a1b..58fcc1edd852 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -707,6 +707,7 @@ secondary_startup:
> >   	/*
> >   	 * Common entry point for secondary CPUs.
> >   	 */
> > +	bl	__cpu_secondary_check52bitva
> >   	bl	__cpu_setup			// initialise processor
> >   	adrp	x1, swapper_pg_dir
> >   	bl	__enable_mmu
> > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> >   	ret
> >   ENDPROC(__enable_mmu)
> > +ENTRY(__cpu_secondary_check52bitva)
> > +#ifdef CONFIG_ARM64_52BIT_VA
> > +	ldr_l	x0, vabits_user
> > +	cmp	x0, #52
> > +	b.ne	2f > +
> > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > +	cbnz	x0, 2f
> > +
> > +	adr_l	x0, va52mismatch
> > +	mov	w1, #1
> > +	strb	w1, [x0]
> > +	dmb	sy
> > +	dc	ivac, x0	// Invalidate potentially stale cache line
> 
> You may have to clear this variable before a CPU is brought up to avoid
> raising a false error message when another secondary CPU doesn't boot
> for some other reason (say granule support) after a CPU failed with lack
> of 52bitva. It is really a crazy corner case.

Can't we just follow the example set by the EL2 setup in the way that is
uses __boot_cpu_mode? In that case, we only need one variable and you can
detect a problem by comparing the two halves.

Will

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> On 12/06/2018 10:50 PM, Steve Capper wrote:
> > For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
> > we have to be careful in allowing secondary CPUs to boot if 52-bit
> > virtual addresses have already been enabled on the boot CPU.
> > 
> > This patch adds code to the secondary startup path. If the boot CPU has
> > enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
> > secondary can also enable 52-bit support. If not, the secondary is
> > prevented from booting and an error message is displayed indicating why.
> > 
> > Technically this patch could be implemented using the cpufeature code
> > when considering 52-bit userspace support. However, we employ low level
> > checks here as the cpufeature code won't be able to run if we have
> > mismatched 52-bit kernel va support.
> > 
> > Signed-off-by: Steve Capper <steve.capper@arm.com>
> > 
> 
> The patch looks good to me, except for one comment below.
> 
> > ---
> > 
> > Patch is new in V5 of the series
> > ---
> >   arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
> >   arch/arm64/kernel/smp.c  |  5 +++++
> >   2 files changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index f60081be9a1b..58fcc1edd852 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -707,6 +707,7 @@ secondary_startup:
> >   	/*
> >   	 * Common entry point for secondary CPUs.
> >   	 */
> > +	bl	__cpu_secondary_check52bitva
> >   	bl	__cpu_setup			// initialise processor
> >   	adrp	x1, swapper_pg_dir
> >   	bl	__enable_mmu
> > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> >   	ret
> >   ENDPROC(__enable_mmu)
> > +ENTRY(__cpu_secondary_check52bitva)
> > +#ifdef CONFIG_ARM64_52BIT_VA
> > +	ldr_l	x0, vabits_user
> > +	cmp	x0, #52
> > +	b.ne	2f > +
> > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > +	cbnz	x0, 2f
> > +
> > +	adr_l	x0, va52mismatch
> > +	mov	w1, #1
> > +	strb	w1, [x0]
> > +	dmb	sy
> > +	dc	ivac, x0	// Invalidate potentially stale cache line
> 
> You may have to clear this variable before a CPU is brought up to avoid
> raising a false error message when another secondary CPU doesn't boot
> for some other reason (say granule support) after a CPU failed with lack
> of 52bitva. It is really a crazy corner case.

Can't we just follow the example set by the EL2 setup in the way that is
uses __boot_cpu_mode? In that case, we only need one variable and you can
detect a problem by comparing the two halves.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

On 07/12/2018 15:26, Will Deacon wrote:
> On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
>> On 12/06/2018 10:50 PM, Steve Capper wrote:
>>> For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
>>> we have to be careful in allowing secondary CPUs to boot if 52-bit
>>> virtual addresses have already been enabled on the boot CPU.
>>>
>>> This patch adds code to the secondary startup path. If the boot CPU has
>>> enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
>>> secondary can also enable 52-bit support. If not, the secondary is
>>> prevented from booting and an error message is displayed indicating why.
>>>
>>> Technically this patch could be implemented using the cpufeature code
>>> when considering 52-bit userspace support. However, we employ low level
>>> checks here as the cpufeature code won't be able to run if we have
>>> mismatched 52-bit kernel va support.
>>>
>>> Signed-off-by: Steve Capper <steve.capper@arm.com>
>>>
>>
>> The patch looks good to me, except for one comment below.
>>
>>> ---
>>>
>>> Patch is new in V5 of the series
>>> ---
>>>    arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
>>>    arch/arm64/kernel/smp.c  |  5 +++++
>>>    2 files changed, 31 insertions(+)
>>>
>>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>>> index f60081be9a1b..58fcc1edd852 100644
>>> --- a/arch/arm64/kernel/head.S
>>> +++ b/arch/arm64/kernel/head.S
>>> @@ -707,6 +707,7 @@ secondary_startup:
>>>    	/*
>>>    	 * Common entry point for secondary CPUs.
>>>    	 */
>>> +	bl	__cpu_secondary_check52bitva
>>>    	bl	__cpu_setup			// initialise processor
>>>    	adrp	x1, swapper_pg_dir
>>>    	bl	__enable_mmu
>>> @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
>>>    	ret
>>>    ENDPROC(__enable_mmu)
>>> +ENTRY(__cpu_secondary_check52bitva)
>>> +#ifdef CONFIG_ARM64_52BIT_VA
>>> +	ldr_l	x0, vabits_user
>>> +	cmp	x0, #52
>>> +	b.ne	2f > +
>>> +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
>>> +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
>>> +	cbnz	x0, 2f
>>> +
>>> +	adr_l	x0, va52mismatch
>>> +	mov	w1, #1
>>> +	strb	w1, [x0]
>>> +	dmb	sy
>>> +	dc	ivac, x0	// Invalidate potentially stale cache line
>>
>> You may have to clear this variable before a CPU is brought up to avoid
>> raising a false error message when another secondary CPU doesn't boot
>> for some other reason (say granule support) after a CPU failed with lack
>> of 52bitva. It is really a crazy corner case.
> 
> Can't we just follow the example set by the EL2 setup in the way that is
> uses __boot_cpu_mode? In that case, we only need one variable and you can
> detect a problem by comparing the two halves.

The only difference here is, the support is bolted at boot CPU time and hence
we need to verify each and every CPU, unlike the __boot_cpu_mode where we
check for mismatch after the SMP CPUs are brought up. If we decide to make
the choice later, something like that could work. The only caveat is the 52bit
kernel VA will have to do something like the above.

Cheers
Suzuki




> 
> Will
> 

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

On 07/12/2018 15:26, Will Deacon wrote:
> On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
>> On 12/06/2018 10:50 PM, Steve Capper wrote:
>>> For cases where there is a mismatch in ARMv8.2-LVA support between CPUs
>>> we have to be careful in allowing secondary CPUs to boot if 52-bit
>>> virtual addresses have already been enabled on the boot CPU.
>>>
>>> This patch adds code to the secondary startup path. If the boot CPU has
>>> enabled 52-bit VAs then ID_AA64MMFR2_EL1 is checked to see if the
>>> secondary can also enable 52-bit support. If not, the secondary is
>>> prevented from booting and an error message is displayed indicating why.
>>>
>>> Technically this patch could be implemented using the cpufeature code
>>> when considering 52-bit userspace support. However, we employ low level
>>> checks here as the cpufeature code won't be able to run if we have
>>> mismatched 52-bit kernel va support.
>>>
>>> Signed-off-by: Steve Capper <steve.capper@arm.com>
>>>
>>
>> The patch looks good to me, except for one comment below.
>>
>>> ---
>>>
>>> Patch is new in V5 of the series
>>> ---
>>>    arch/arm64/kernel/head.S | 26 ++++++++++++++++++++++++++
>>>    arch/arm64/kernel/smp.c  |  5 +++++
>>>    2 files changed, 31 insertions(+)
>>>
>>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>>> index f60081be9a1b..58fcc1edd852 100644
>>> --- a/arch/arm64/kernel/head.S
>>> +++ b/arch/arm64/kernel/head.S
>>> @@ -707,6 +707,7 @@ secondary_startup:
>>>    	/*
>>>    	 * Common entry point for secondary CPUs.
>>>    	 */
>>> +	bl	__cpu_secondary_check52bitva
>>>    	bl	__cpu_setup			// initialise processor
>>>    	adrp	x1, swapper_pg_dir
>>>    	bl	__enable_mmu
>>> @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
>>>    	ret
>>>    ENDPROC(__enable_mmu)
>>> +ENTRY(__cpu_secondary_check52bitva)
>>> +#ifdef CONFIG_ARM64_52BIT_VA
>>> +	ldr_l	x0, vabits_user
>>> +	cmp	x0, #52
>>> +	b.ne	2f > +
>>> +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
>>> +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
>>> +	cbnz	x0, 2f
>>> +
>>> +	adr_l	x0, va52mismatch
>>> +	mov	w1, #1
>>> +	strb	w1, [x0]
>>> +	dmb	sy
>>> +	dc	ivac, x0	// Invalidate potentially stale cache line
>>
>> You may have to clear this variable before a CPU is brought up to avoid
>> raising a false error message when another secondary CPU doesn't boot
>> for some other reason (say granule support) after a CPU failed with lack
>> of 52bitva. It is really a crazy corner case.
> 
> Can't we just follow the example set by the EL2 setup in the way that is
> uses __boot_cpu_mode? In that case, we only need one variable and you can
> detect a problem by comparing the two halves.

The only difference here is, the support is bolted at boot CPU time and hence
we need to verify each and every CPU, unlike the __boot_cpu_mode where we
check for mismatch after the SMP CPUs are brought up. If we decide to make
the choice later, something like that could work. The only caveat is the 52bit
kernel VA will have to do something like the above.

Cheers
Suzuki




> 
> Will
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 1/7] mm: mmap: Allow for "high" userspace addresses

[Andrew Morton]

On Thu,  6 Dec 2018 22:50:36 +0000 Steve Capper <steve.capper@arm.com> wrote:

> This patch adds support for "high" userspace addresses that are
> optionally supported on the system and have to be requested via a hint
> mechanism ("high" addr parameter to mmap).
> 
> Architectures such as powerpc and x86 achieve this by making changes to
> their architectural versions of arch_get_unmapped_* functions. However,
> on arm64 we use the generic versions of these functions.
> 
> Rather than duplicate the generic arch_get_unmapped_* implementations
> for arm64, this patch instead introduces two architectural helper macros
> and applies them to arch_get_unmapped_*:
>  arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint
>  arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint
> 
> If these macros are not defined in architectural code then they default
> to (TASK_SIZE) and (base) so should not introduce any behavioural
> changes to architectures that do not define them.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Acked-by: Andrew Morton <akpm@linux-foundation.org>

Re: [PATCH V5 1/7] mm: mmap: Allow for "high" userspace addresses

[Andrew Morton]

On Thu,  6 Dec 2018 22:50:36 +0000 Steve Capper <steve.capper@arm.com> wrote:

> This patch adds support for "high" userspace addresses that are
> optionally supported on the system and have to be requested via a hint
> mechanism ("high" addr parameter to mmap).
> 
> Architectures such as powerpc and x86 achieve this by making changes to
> their architectural versions of arch_get_unmapped_* functions. However,
> on arm64 we use the generic versions of these functions.
> 
> Rather than duplicate the generic arch_get_unmapped_* implementations
> for arm64, this patch instead introduces two architectural helper macros
> and applies them to arch_get_unmapped_*:
>  arch_get_mmap_end(addr) - get mmap upper limit depending on addr hint
>  arch_get_mmap_base(addr, base) - get mmap_base depending on addr hint
> 
> If these macros are not defined in architectural code then they default
> to (TASK_SIZE) and (base) so should not introduce any behavioural
> changes to architectures that do not define them.
> 
> Signed-off-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Acked-by: Andrew Morton <akpm@linux-foundation.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> 
> 
> On 07/12/2018 15:26, Will Deacon wrote:
> > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > index f60081be9a1b..58fcc1edd852 100644
> > > > --- a/arch/arm64/kernel/head.S
> > > > +++ b/arch/arm64/kernel/head.S
> > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > >    	/*
> > > >    	 * Common entry point for secondary CPUs.
> > > >    	 */
> > > > +	bl	__cpu_secondary_check52bitva
> > > >    	bl	__cpu_setup			// initialise processor
> > > >    	adrp	x1, swapper_pg_dir
> > > >    	bl	__enable_mmu
> > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > >    	ret
> > > >    ENDPROC(__enable_mmu)
> > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > +	ldr_l	x0, vabits_user
> > > > +	cmp	x0, #52
> > > > +	b.ne	2f > +
> > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > +	cbnz	x0, 2f
> > > > +
> > > > +	adr_l	x0, va52mismatch
> > > > +	mov	w1, #1
> > > > +	strb	w1, [x0]
> > > > +	dmb	sy
> > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > 
> > > You may have to clear this variable before a CPU is brought up to avoid
> > > raising a false error message when another secondary CPU doesn't boot
> > > for some other reason (say granule support) after a CPU failed with lack
> > > of 52bitva. It is really a crazy corner case.
> > 
> > Can't we just follow the example set by the EL2 setup in the way that is
> > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > detect a problem by comparing the two halves.
> 
> The only difference here is, the support is bolted at boot CPU time and hence
> we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> check for mismatch after the SMP CPUs are brought up. If we decide to make
> the choice later, something like that could work. The only caveat is the 52bit
> kernel VA will have to do something like the above.

So looking at this a bit more, I think we're better off repurposing the
upper bits of the early boot status word to contain a reason code, rather
than introducing new variables for every possible mismatch.

Does the untested diff below look remotely sane to you?

Will

--->8

diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index f82b447bd34f..1895561839a9 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -17,15 +17,20 @@
 #define __ASM_SMP_H
 
 /* Values for secondary_data.status */
+#define CPU_STUCK_REASON_SHIFT		(8)
+#define CPU_BOOT_STATUS_MASK		((1U << CPU_STUCK_REASON_SHIFT) - 1)
 
-#define CPU_MMU_OFF		(-1)
-#define CPU_BOOT_SUCCESS	(0)
+#define CPU_MMU_OFF			(-1)
+#define CPU_BOOT_SUCCESS		(0)
 /* The cpu invoked ops->cpu_die, synchronise it with cpu_kill */
-#define CPU_KILL_ME		(1)
+#define CPU_KILL_ME			(1)
 /* The cpu couldn't die gracefully and is looping in the kernel */
-#define CPU_STUCK_IN_KERNEL	(2)
+#define CPU_STUCK_IN_KERNEL		(2)
 /* Fatal system error detected by secondary CPU, crash the system */
-#define CPU_PANIC_KERNEL	(3)
+#define CPU_PANIC_KERNEL		(3)
+
+#define CPU_STUCK_REASON_52_BIT_VA	(1U << CPU_STUCK_REASON_SHIFT)
+#define CPU_STUCK_REASON_NO_GRAN	(2U << CPU_STUCK_REASON_SHIFT)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index c229d9cfe9bf..7377e14884e3 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -809,13 +809,8 @@ ENTRY(__cpu_secondary_check52bitva)
 	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
 	cbnz	x0, 2f
 
-	adr_l	x0, va52mismatch
-	mov	w1, #1
-	strb	w1, [x0]
-	dmb	sy
-	dc	ivac, x0	// Invalidate potentially stale cache line
-
-	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x0, x1
+	update_early_cpu_boot_status \
+		CPU_STUCK_IN_KERNEL | CPU_STUCK_REASON_52_BIT_VA, x0, x1
 1:	wfe
 	wfi
 	b	1b
@@ -826,7 +821,8 @@ ENDPROC(__cpu_secondary_check52bitva)
 
 __no_granule_support:
 	/* Indicate that this CPU can't boot and is stuck in the kernel */
-	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x1, x2
+	update_early_cpu_boot_status \
+		CPU_STUCK_IN_KERNEL | CPU_STUCK_REASON_NO_GRAN, x1, x2
 1:
 	wfe
 	wfi
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index e15b0b64d4d0..4e3bfbde829a 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -108,7 +108,6 @@ static int boot_secondary(unsigned int cpu, struct task_struct *idle)
 }
 
 static DECLARE_COMPLETION(cpu_running);
-bool va52mismatch __ro_after_init;
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)
 {
@@ -138,10 +137,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 
 		if (!cpu_online(cpu)) {
 			pr_crit("CPU%u: failed to come online\n", cpu);
-
-			if (IS_ENABLED(CONFIG_ARM64_52BIT_VA) && va52mismatch)
-				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
-
 			ret = -EIO;
 		}
 	} else {
@@ -156,7 +151,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		if (status == CPU_MMU_OFF)
 			status = READ_ONCE(__early_cpu_boot_status);
 
-		switch (status) {
+		switch (status & CPU_BOOT_STATUS_MASK) {
 		default:
 			pr_err("CPU%u: failed in unknown state : 0x%lx\n",
 					cpu, status);
@@ -170,6 +165,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 			pr_crit("CPU%u: may not have shut down cleanly\n", cpu);
 		case CPU_STUCK_IN_KERNEL:
 			pr_crit("CPU%u: is stuck in kernel\n", cpu);
+			if (status & CPU_STUCK_REASON_52_BIT_VA)
+				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
+			if (status & CPU_STUCK_REASON_NO_GRAN)
+				pr_crit("CPU%u: does not support %luK granule \n", cpu, PAGE_SIZE / SZ_1K);
 			cpus_stuck_in_kernel++;
 			break;
 		case CPU_PANIC_KERNEL:

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> 
> 
> On 07/12/2018 15:26, Will Deacon wrote:
> > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > index f60081be9a1b..58fcc1edd852 100644
> > > > --- a/arch/arm64/kernel/head.S
> > > > +++ b/arch/arm64/kernel/head.S
> > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > >    	/*
> > > >    	 * Common entry point for secondary CPUs.
> > > >    	 */
> > > > +	bl	__cpu_secondary_check52bitva
> > > >    	bl	__cpu_setup			// initialise processor
> > > >    	adrp	x1, swapper_pg_dir
> > > >    	bl	__enable_mmu
> > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > >    	ret
> > > >    ENDPROC(__enable_mmu)
> > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > +	ldr_l	x0, vabits_user
> > > > +	cmp	x0, #52
> > > > +	b.ne	2f > +
> > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > +	cbnz	x0, 2f
> > > > +
> > > > +	adr_l	x0, va52mismatch
> > > > +	mov	w1, #1
> > > > +	strb	w1, [x0]
> > > > +	dmb	sy
> > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > 
> > > You may have to clear this variable before a CPU is brought up to avoid
> > > raising a false error message when another secondary CPU doesn't boot
> > > for some other reason (say granule support) after a CPU failed with lack
> > > of 52bitva. It is really a crazy corner case.
> > 
> > Can't we just follow the example set by the EL2 setup in the way that is
> > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > detect a problem by comparing the two halves.
> 
> The only difference here is, the support is bolted at boot CPU time and hence
> we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> check for mismatch after the SMP CPUs are brought up. If we decide to make
> the choice later, something like that could work. The only caveat is the 52bit
> kernel VA will have to do something like the above.

So looking at this a bit more, I think we're better off repurposing the
upper bits of the early boot status word to contain a reason code, rather
than introducing new variables for every possible mismatch.

Does the untested diff below look remotely sane to you?

Will

--->8

diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index f82b447bd34f..1895561839a9 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -17,15 +17,20 @@
 #define __ASM_SMP_H
 
 /* Values for secondary_data.status */
+#define CPU_STUCK_REASON_SHIFT		(8)
+#define CPU_BOOT_STATUS_MASK		((1U << CPU_STUCK_REASON_SHIFT) - 1)
 
-#define CPU_MMU_OFF		(-1)
-#define CPU_BOOT_SUCCESS	(0)
+#define CPU_MMU_OFF			(-1)
+#define CPU_BOOT_SUCCESS		(0)
 /* The cpu invoked ops->cpu_die, synchronise it with cpu_kill */
-#define CPU_KILL_ME		(1)
+#define CPU_KILL_ME			(1)
 /* The cpu couldn't die gracefully and is looping in the kernel */
-#define CPU_STUCK_IN_KERNEL	(2)
+#define CPU_STUCK_IN_KERNEL		(2)
 /* Fatal system error detected by secondary CPU, crash the system */
-#define CPU_PANIC_KERNEL	(3)
+#define CPU_PANIC_KERNEL		(3)
+
+#define CPU_STUCK_REASON_52_BIT_VA	(1U << CPU_STUCK_REASON_SHIFT)
+#define CPU_STUCK_REASON_NO_GRAN	(2U << CPU_STUCK_REASON_SHIFT)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index c229d9cfe9bf..7377e14884e3 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -809,13 +809,8 @@ ENTRY(__cpu_secondary_check52bitva)
 	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
 	cbnz	x0, 2f
 
-	adr_l	x0, va52mismatch
-	mov	w1, #1
-	strb	w1, [x0]
-	dmb	sy
-	dc	ivac, x0	// Invalidate potentially stale cache line
-
-	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x0, x1
+	update_early_cpu_boot_status \
+		CPU_STUCK_IN_KERNEL | CPU_STUCK_REASON_52_BIT_VA, x0, x1
 1:	wfe
 	wfi
 	b	1b
@@ -826,7 +821,8 @@ ENDPROC(__cpu_secondary_check52bitva)
 
 __no_granule_support:
 	/* Indicate that this CPU can't boot and is stuck in the kernel */
-	update_early_cpu_boot_status CPU_STUCK_IN_KERNEL, x1, x2
+	update_early_cpu_boot_status \
+		CPU_STUCK_IN_KERNEL | CPU_STUCK_REASON_NO_GRAN, x1, x2
 1:
 	wfe
 	wfi
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index e15b0b64d4d0..4e3bfbde829a 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -108,7 +108,6 @@ static int boot_secondary(unsigned int cpu, struct task_struct *idle)
 }
 
 static DECLARE_COMPLETION(cpu_running);
-bool va52mismatch __ro_after_init;
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)
 {
@@ -138,10 +137,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 
 		if (!cpu_online(cpu)) {
 			pr_crit("CPU%u: failed to come online\n", cpu);
-
-			if (IS_ENABLED(CONFIG_ARM64_52BIT_VA) && va52mismatch)
-				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
-
 			ret = -EIO;
 		}
 	} else {
@@ -156,7 +151,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		if (status == CPU_MMU_OFF)
 			status = READ_ONCE(__early_cpu_boot_status);
 
-		switch (status) {
+		switch (status & CPU_BOOT_STATUS_MASK) {
 		default:
 			pr_err("CPU%u: failed in unknown state : 0x%lx\n",
 					cpu, status);
@@ -170,6 +165,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 			pr_crit("CPU%u: may not have shut down cleanly\n", cpu);
 		case CPU_STUCK_IN_KERNEL:
 			pr_crit("CPU%u: is stuck in kernel\n", cpu);
+			if (status & CPU_STUCK_REASON_52_BIT_VA)
+				pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
+			if (status & CPU_STUCK_REASON_NO_GRAN)
+				pr_crit("CPU%u: does not support %luK granule \n", cpu, PAGE_SIZE / SZ_1K);
 			cpus_stuck_in_kernel++;
 			break;
 		case CPU_PANIC_KERNEL:

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > 
> > 
> > On 07/12/2018 15:26, Will Deacon wrote:
> > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > --- a/arch/arm64/kernel/head.S
> > > > > +++ b/arch/arm64/kernel/head.S
> > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > >    	/*
> > > > >    	 * Common entry point for secondary CPUs.
> > > > >    	 */
> > > > > +	bl	__cpu_secondary_check52bitva
> > > > >    	bl	__cpu_setup			// initialise processor
> > > > >    	adrp	x1, swapper_pg_dir
> > > > >    	bl	__enable_mmu
> > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > >    	ret
> > > > >    ENDPROC(__enable_mmu)
> > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > +	ldr_l	x0, vabits_user
> > > > > +	cmp	x0, #52
> > > > > +	b.ne	2f > +
> > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > +	cbnz	x0, 2f
> > > > > +
> > > > > +	adr_l	x0, va52mismatch
> > > > > +	mov	w1, #1
> > > > > +	strb	w1, [x0]
> > > > > +	dmb	sy
> > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > 
> > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > raising a false error message when another secondary CPU doesn't boot
> > > > for some other reason (say granule support) after a CPU failed with lack
> > > > of 52bitva. It is really a crazy corner case.
> > > 
> > > Can't we just follow the example set by the EL2 setup in the way that is
> > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > detect a problem by comparing the two halves.
> > 
> > The only difference here is, the support is bolted at boot CPU time and hence
> > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > the choice later, something like that could work. The only caveat is the 52bit
> > kernel VA will have to do something like the above.
> 
> So looking at this a bit more, I think we're better off repurposing the
> upper bits of the early boot status word to contain a reason code, rather
> than introducing new variables for every possible mismatch.
> 
> Does the untested diff below look remotely sane to you?
> 
> Will
> 

Thanks Will,
This looks good to me, I will test now and fold this into a patch.

Cheers,
-- 
Steve

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > 
> > 
> > On 07/12/2018 15:26, Will Deacon wrote:
> > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > --- a/arch/arm64/kernel/head.S
> > > > > +++ b/arch/arm64/kernel/head.S
> > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > >    	/*
> > > > >    	 * Common entry point for secondary CPUs.
> > > > >    	 */
> > > > > +	bl	__cpu_secondary_check52bitva
> > > > >    	bl	__cpu_setup			// initialise processor
> > > > >    	adrp	x1, swapper_pg_dir
> > > > >    	bl	__enable_mmu
> > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > >    	ret
> > > > >    ENDPROC(__enable_mmu)
> > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > +	ldr_l	x0, vabits_user
> > > > > +	cmp	x0, #52
> > > > > +	b.ne	2f > +
> > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > +	cbnz	x0, 2f
> > > > > +
> > > > > +	adr_l	x0, va52mismatch
> > > > > +	mov	w1, #1
> > > > > +	strb	w1, [x0]
> > > > > +	dmb	sy
> > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > 
> > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > raising a false error message when another secondary CPU doesn't boot
> > > > for some other reason (say granule support) after a CPU failed with lack
> > > > of 52bitva. It is really a crazy corner case.
> > > 
> > > Can't we just follow the example set by the EL2 setup in the way that is
> > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > detect a problem by comparing the two halves.
> > 
> > The only difference here is, the support is bolted at boot CPU time and hence
> > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > the choice later, something like that could work. The only caveat is the 52bit
> > kernel VA will have to do something like the above.
> 
> So looking at this a bit more, I think we're better off repurposing the
> upper bits of the early boot status word to contain a reason code, rather
> than introducing new variables for every possible mismatch.
> 
> Does the untested diff below look remotely sane to you?
> 
> Will
> 

Thanks Will,
This looks good to me, I will test now and fold this into a patch.

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > >    	/*
> > > > > >    	 * Common entry point for secondary CPUs.
> > > > > >    	 */
> > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > >    	adrp	x1, swapper_pg_dir
> > > > > >    	bl	__enable_mmu
> > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > >    	ret
> > > > > >    ENDPROC(__enable_mmu)
> > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > +	ldr_l	x0, vabits_user
> > > > > > +	cmp	x0, #52
> > > > > > +	b.ne	2f > +
> > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > +	cbnz	x0, 2f
> > > > > > +
> > > > > > +	adr_l	x0, va52mismatch
> > > > > > +	mov	w1, #1
> > > > > > +	strb	w1, [x0]
> > > > > > +	dmb	sy
> > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > 
> > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > of 52bitva. It is really a crazy corner case.
> > > > 
> > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > detect a problem by comparing the two halves.
> > > 
> > > The only difference here is, the support is bolted at boot CPU time and hence
> > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > the choice later, something like that could work. The only caveat is the 52bit
> > > kernel VA will have to do something like the above.
> > 
> > So looking at this a bit more, I think we're better off repurposing the
> > upper bits of the early boot status word to contain a reason code, rather
> > than introducing new variables for every possible mismatch.
> > 
> > Does the untested diff below look remotely sane to you?
> > 
> > Will
> > 
> 
> Thanks Will,
> This looks good to me, I will test now and fold this into a patch.

Cheers, Steve. Testing would be handy, but don't worry about respinning the
patches as I'm already on top of this and hope to push this out later today.

Will

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Will Deacon]

On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > >    	/*
> > > > > >    	 * Common entry point for secondary CPUs.
> > > > > >    	 */
> > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > >    	adrp	x1, swapper_pg_dir
> > > > > >    	bl	__enable_mmu
> > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > >    	ret
> > > > > >    ENDPROC(__enable_mmu)
> > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > +	ldr_l	x0, vabits_user
> > > > > > +	cmp	x0, #52
> > > > > > +	b.ne	2f > +
> > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > +	cbnz	x0, 2f
> > > > > > +
> > > > > > +	adr_l	x0, va52mismatch
> > > > > > +	mov	w1, #1
> > > > > > +	strb	w1, [x0]
> > > > > > +	dmb	sy
> > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > 
> > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > of 52bitva. It is really a crazy corner case.
> > > > 
> > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > detect a problem by comparing the two halves.
> > > 
> > > The only difference here is, the support is bolted at boot CPU time and hence
> > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > the choice later, something like that could work. The only caveat is the 52bit
> > > kernel VA will have to do something like the above.
> > 
> > So looking at this a bit more, I think we're better off repurposing the
> > upper bits of the early boot status word to contain a reason code, rather
> > than introducing new variables for every possible mismatch.
> > 
> > Does the untested diff below look remotely sane to you?
> > 
> > Will
> > 
> 
> Thanks Will,
> This looks good to me, I will test now and fold this into a patch.

Cheers, Steve. Testing would be handy, but don't worry about respinning the
patches as I'm already on top of this and hope to push this out later today.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > >    	/*
> > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > >    	 */
> > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > >    	bl	__enable_mmu
> > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > >    	ret
> > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > +	cmp	x0, #52
> > > > > > > +	b.ne	2f > +
> > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > +	cbnz	x0, 2f
> > > > > > > +
> > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > +	mov	w1, #1
> > > > > > > +	strb	w1, [x0]
> > > > > > > +	dmb	sy
> > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > 
> > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > of 52bitva. It is really a crazy corner case.
> > > > > 
> > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > detect a problem by comparing the two halves.
> > > > 
> > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > kernel VA will have to do something like the above.
> > > 
> > > So looking at this a bit more, I think we're better off repurposing the
> > > upper bits of the early boot status word to contain a reason code, rather
> > > than introducing new variables for every possible mismatch.
> > > 
> > > Does the untested diff below look remotely sane to you?
> > > 
> > > Will
> > > 
> > 
> > Thanks Will,
> > This looks good to me, I will test now and fold this into a patch.
> 
> Cheers, Steve. Testing would be handy, but don't worry about respinning the
> patches as I'm already on top of this and hope to push this out later today.
> 

Thanks Will,
This looks good to me so FWIW:
Tested-by: Steve Capper <steve.capper@arm.com>

(for both the 52-bit VA mismatch and 64KB granule not supported cases
using the model).

The only small issue I see is that if subsequent CPUs aren't brought
online (because they don't exist in the model) then the error reason is
repeated.

I'll dig into this.

Cheers,
-- 
Steve

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > >    	/*
> > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > >    	 */
> > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > >    	bl	__enable_mmu
> > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > >    	ret
> > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > +	cmp	x0, #52
> > > > > > > +	b.ne	2f > +
> > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > +	cbnz	x0, 2f
> > > > > > > +
> > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > +	mov	w1, #1
> > > > > > > +	strb	w1, [x0]
> > > > > > > +	dmb	sy
> > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > 
> > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > of 52bitva. It is really a crazy corner case.
> > > > > 
> > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > detect a problem by comparing the two halves.
> > > > 
> > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > kernel VA will have to do something like the above.
> > > 
> > > So looking at this a bit more, I think we're better off repurposing the
> > > upper bits of the early boot status word to contain a reason code, rather
> > > than introducing new variables for every possible mismatch.
> > > 
> > > Does the untested diff below look remotely sane to you?
> > > 
> > > Will
> > > 
> > 
> > Thanks Will,
> > This looks good to me, I will test now and fold this into a patch.
> 
> Cheers, Steve. Testing would be handy, but don't worry about respinning the
> patches as I'm already on top of this and hope to push this out later today.
> 

Thanks Will,
This looks good to me so FWIW:
Tested-by: Steve Capper <steve.capper@arm.com>

(for both the 52-bit VA mismatch and 64KB granule not supported cases
using the model).

The only small issue I see is that if subsequent CPUs aren't brought
online (because they don't exist in the model) then the error reason is
repeated.

I'll dig into this.

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > >    	/*
> > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > >    	 */
> > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > >    	bl	__enable_mmu
> > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > >    	ret
> > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > +	cmp	x0, #52
> > > > > > > > +	b.ne	2f > +
> > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > +	cbnz	x0, 2f
> > > > > > > > +
> > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > +	mov	w1, #1
> > > > > > > > +	strb	w1, [x0]
> > > > > > > > +	dmb	sy
> > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > 
> > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > 
> > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > detect a problem by comparing the two halves.
> > > > > 
> > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > kernel VA will have to do something like the above.
> > > > 
> > > > So looking at this a bit more, I think we're better off repurposing the
> > > > upper bits of the early boot status word to contain a reason code, rather
> > > > than introducing new variables for every possible mismatch.
> > > > 
> > > > Does the untested diff below look remotely sane to you?
> > > > 
> > > > Will
> > > > 
> > > 
> > > Thanks Will,
> > > This looks good to me, I will test now and fold this into a patch.
> > 
> > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > patches as I'm already on top of this and hope to push this out later today.
> > 
> 
> Thanks Will,
> This looks good to me so FWIW:
> Tested-by: Steve Capper <steve.capper@arm.com>
> 
> (for both the 52-bit VA mismatch and 64KB granule not supported cases
> using the model).
> 
> The only small issue I see is that if subsequent CPUs aren't brought
> online (because they don't exist in the model) then the error reason is
> repeated.
> 
> I'll dig into this.
>

I think __early_cpu_boot_status needs to be reset at the beginning of
__cpu_up before the secondary is booted. Testing a check for this now.

Cheers,
-- 
Steve

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > >    	/*
> > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > >    	 */
> > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > >    	bl	__enable_mmu
> > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > >    	ret
> > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > +	cmp	x0, #52
> > > > > > > > +	b.ne	2f > +
> > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > +	cbnz	x0, 2f
> > > > > > > > +
> > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > +	mov	w1, #1
> > > > > > > > +	strb	w1, [x0]
> > > > > > > > +	dmb	sy
> > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > 
> > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > 
> > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > detect a problem by comparing the two halves.
> > > > > 
> > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > kernel VA will have to do something like the above.
> > > > 
> > > > So looking at this a bit more, I think we're better off repurposing the
> > > > upper bits of the early boot status word to contain a reason code, rather
> > > > than introducing new variables for every possible mismatch.
> > > > 
> > > > Does the untested diff below look remotely sane to you?
> > > > 
> > > > Will
> > > > 
> > > 
> > > Thanks Will,
> > > This looks good to me, I will test now and fold this into a patch.
> > 
> > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > patches as I'm already on top of this and hope to push this out later today.
> > 
> 
> Thanks Will,
> This looks good to me so FWIW:
> Tested-by: Steve Capper <steve.capper@arm.com>
> 
> (for both the 52-bit VA mismatch and 64KB granule not supported cases
> using the model).
> 
> The only small issue I see is that if subsequent CPUs aren't brought
> online (because they don't exist in the model) then the error reason is
> repeated.
> 
> I'll dig into this.
>

I think __early_cpu_boot_status needs to be reset at the beginning of
__cpu_up before the secondary is booted. Testing a check for this now.

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 05:08:31PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > > >    	/*
> > > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > > >    	 */
> > > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > > >    	bl	__enable_mmu
> > > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > > >    	ret
> > > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > > +	cmp	x0, #52
> > > > > > > > > +	b.ne	2f > +
> > > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > > +	cbnz	x0, 2f
> > > > > > > > > +
> > > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > > +	mov	w1, #1
> > > > > > > > > +	strb	w1, [x0]
> > > > > > > > > +	dmb	sy
> > > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > > 
> > > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > > 
> > > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > > detect a problem by comparing the two halves.
> > > > > > 
> > > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > > kernel VA will have to do something like the above.
> > > > > 
> > > > > So looking at this a bit more, I think we're better off repurposing the
> > > > > upper bits of the early boot status word to contain a reason code, rather
> > > > > than introducing new variables for every possible mismatch.
> > > > > 
> > > > > Does the untested diff below look remotely sane to you?
> > > > > 
> > > > > Will
> > > > > 
> > > > 
> > > > Thanks Will,
> > > > This looks good to me, I will test now and fold this into a patch.
> > > 
> > > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > > patches as I'm already on top of this and hope to push this out later today.
> > > 
> > 
> > Thanks Will,
> > This looks good to me so FWIW:
> > Tested-by: Steve Capper <steve.capper@arm.com>
> > 
> > (for both the 52-bit VA mismatch and 64KB granule not supported cases
> > using the model).
> > 
> > The only small issue I see is that if subsequent CPUs aren't brought
> > online (because they don't exist in the model) then the error reason is
> > repeated.
> > 
> > I'll dig into this.
> >
> 
> I think __early_cpu_boot_status needs to be reset at the beginning of
> __cpu_up before the secondary is booted. Testing a check for this now.
>

Hi Will,

The following fixed the repeating error message problem for me. If you
want, I can send a separate patch to fix this?

Cheers,
-- 
Steve


--->8

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 4e3bfbde829a..936156a7ae88 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -123,6 +123,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 	update_cpu_boot_status(CPU_MMU_OFF);
 	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
 
+	__early_cpu_boot_status = 0;
+	dsb(ishst);
+	__flush_dcache_area(&__early_cpu_boot_status,
+			sizeof(__early_cpu_boot_status));
+
 	/*
 	 * Now bring the CPU into our world.
 	 */

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Steve Capper]

On Mon, Dec 10, 2018 at 05:08:31PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > > >    	/*
> > > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > > >    	 */
> > > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > > >    	bl	__enable_mmu
> > > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > > >    	ret
> > > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > > +	cmp	x0, #52
> > > > > > > > > +	b.ne	2f > +
> > > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > > +	cbnz	x0, 2f
> > > > > > > > > +
> > > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > > +	mov	w1, #1
> > > > > > > > > +	strb	w1, [x0]
> > > > > > > > > +	dmb	sy
> > > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > > 
> > > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > > 
> > > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > > detect a problem by comparing the two halves.
> > > > > > 
> > > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > > kernel VA will have to do something like the above.
> > > > > 
> > > > > So looking at this a bit more, I think we're better off repurposing the
> > > > > upper bits of the early boot status word to contain a reason code, rather
> > > > > than introducing new variables for every possible mismatch.
> > > > > 
> > > > > Does the untested diff below look remotely sane to you?
> > > > > 
> > > > > Will
> > > > > 
> > > > 
> > > > Thanks Will,
> > > > This looks good to me, I will test now and fold this into a patch.
> > > 
> > > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > > patches as I'm already on top of this and hope to push this out later today.
> > > 
> > 
> > Thanks Will,
> > This looks good to me so FWIW:
> > Tested-by: Steve Capper <steve.capper@arm.com>
> > 
> > (for both the 52-bit VA mismatch and 64KB granule not supported cases
> > using the model).
> > 
> > The only small issue I see is that if subsequent CPUs aren't brought
> > online (because they don't exist in the model) then the error reason is
> > repeated.
> > 
> > I'll dig into this.
> >
> 
> I think __early_cpu_boot_status needs to be reset at the beginning of
> __cpu_up before the secondary is booted. Testing a check for this now.
>

Hi Will,

The following fixed the repeating error message problem for me. If you
want, I can send a separate patch to fix this?

Cheers,
-- 
Steve


--->8

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 4e3bfbde829a..936156a7ae88 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -123,6 +123,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 	update_cpu_boot_status(CPU_MMU_OFF);
 	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
 
+	__early_cpu_boot_status = 0;
+	dsb(ishst);
+	__flush_dcache_area(&__early_cpu_boot_status,
+			sizeof(__early_cpu_boot_status));
+
 	/*
 	 * Now bring the CPU into our world.
 	 */


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

On Mon, Dec 10, 2018 at 05:42:45PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 05:08:31PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> > > On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > > > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > > > >    	/*
> > > > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > > > >    	 */
> > > > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > > > >    	bl	__enable_mmu
> > > > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > > > >    	ret
> > > > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > > > +	cmp	x0, #52
> > > > > > > > > > +	b.ne	2f > +
> > > > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > > > +	cbnz	x0, 2f
> > > > > > > > > > +
> > > > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > > > +	mov	w1, #1
> > > > > > > > > > +	strb	w1, [x0]
> > > > > > > > > > +	dmb	sy
> > > > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > > > 
> > > > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > > > 
> > > > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > > > detect a problem by comparing the two halves.
> > > > > > > 
> > > > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > > > kernel VA will have to do something like the above.
> > > > > > 
> > > > > > So looking at this a bit more, I think we're better off repurposing the
> > > > > > upper bits of the early boot status word to contain a reason code, rather
> > > > > > than introducing new variables for every possible mismatch.
> > > > > > 
> > > > > > Does the untested diff below look remotely sane to you?
> > > > > > 
> > > > > > Will
> > > > > > 
> > > > > 
> > > > > Thanks Will,
> > > > > This looks good to me, I will test now and fold this into a patch.
> > > > 
> > > > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > > > patches as I'm already on top of this and hope to push this out later today.
> > > > 
> > > 
> > > Thanks Will,
> > > This looks good to me so FWIW:
> > > Tested-by: Steve Capper <steve.capper@arm.com>
> > > 
> > > (for both the 52-bit VA mismatch and 64KB granule not supported cases
> > > using the model).
> > > 
> > > The only small issue I see is that if subsequent CPUs aren't brought
> > > online (because they don't exist in the model) then the error reason is
> > > repeated.
> > > 
> > > I'll dig into this.
> > >
> > 
> > I think __early_cpu_boot_status needs to be reset at the beginning of
> > __cpu_up before the secondary is booted. Testing a check for this now.
> >
> 




> Hi Will,
> 
> The following fixed the repeating error message problem for me. If you
> want, I can send a separate patch to fix this?
> 
> Cheers,
> -- 
> Steve
> 
> 
> --->8
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 4e3bfbde829a..936156a7ae88 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -123,6 +123,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  	update_cpu_boot_status(CPU_MMU_OFF);
>  	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
>  
> +	__early_cpu_boot_status = 0;
> +	dsb(ishst);
> +	__flush_dcache_area(&__early_cpu_boot_status,
> +			sizeof(__early_cpu_boot_status));
> +
>  	/*
>  	 * Now bring the CPU into our world.
>  	 */
>

I have tested Will's changes and hit the issue reported by Steve. But this
is mainly due to a bug in our __cpu_up() code, which ignores the errors reported
by the firmware and goes ahead assuming that the CPU entered the kernel and
failed so in the process.

e.g, with a missing CPU3:

[   78.050880] psci: failed to boot CPU3 (-22)
[   78.051079] CPU3: failed to boot: -22
[   78.051319] CPU3: is stuck in kernel
[   78.051496] CPU3: does not support 52-bit VAs

With the fix attached below, I get:
# echo 1 > cpu5/online
[  101.883862] CPU5: failed to come online
[  101.884860] CPU5: is stuck in kernel
[  101.885060] CPU5: does not support 52-bit VAs
-sh: echo: write error: Input/output error
# echo 1 > cpu3/online
[  106.746141] psci: failed to boot CPU3 (-22)
[  106.746360] CPU3: failed to boot: -22
-sh: echo: write error: Invalid argument


----8>----

arm64: smp: Handle errors reported by the firmware

The __cpu_up() routine ignores the errors reported by the firmware
for a CPU bringup operation and looks for the error status set by the
booting CPU. If the CPU never entered the kernel, we could end up
in assuming stale error status, which otherwise would have been
set/cleared appropriately by the booting CPU.

Reported-by: Steve Capper <steve.capper@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/smp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index ac73d6c..a854e3d 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -141,6 +141,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		}
 	} else {
 		pr_err("CPU%u: failed to boot: %d\n", cpu, ret);
+		return ret;
 	}
 
 	secondary_data.task = NULL;
-- 
2.7.4

 

Re: [PATCH V5 5/7] arm64: mm: Prevent mismatched 52-bit VA support

[Suzuki K Poulose]

On Mon, Dec 10, 2018 at 05:42:45PM +0000, Steve Capper wrote:
> On Mon, Dec 10, 2018 at 05:08:31PM +0000, Steve Capper wrote:
> > On Mon, Dec 10, 2018 at 04:55:38PM +0000, Steve Capper wrote:
> > > On Mon, Dec 10, 2018 at 04:18:26PM +0000, Will Deacon wrote:
> > > > On Mon, Dec 10, 2018 at 04:04:02PM +0000, Steve Capper wrote:
> > > > > On Mon, Dec 10, 2018 at 01:36:40PM +0000, Will Deacon wrote:
> > > > > > On Fri, Dec 07, 2018 at 05:28:58PM +0000, Suzuki K Poulose wrote:
> > > > > > > On 07/12/2018 15:26, Will Deacon wrote:
> > > > > > > > On Fri, Dec 07, 2018 at 10:47:57AM +0000, Suzuki K Poulose wrote:
> > > > > > > > > On 12/06/2018 10:50 PM, Steve Capper wrote:
> > > > > > > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > > > > > > > > index f60081be9a1b..58fcc1edd852 100644
> > > > > > > > > > --- a/arch/arm64/kernel/head.S
> > > > > > > > > > +++ b/arch/arm64/kernel/head.S
> > > > > > > > > > @@ -707,6 +707,7 @@ secondary_startup:
> > > > > > > > > >    	/*
> > > > > > > > > >    	 * Common entry point for secondary CPUs.
> > > > > > > > > >    	 */
> > > > > > > > > > +	bl	__cpu_secondary_check52bitva
> > > > > > > > > >    	bl	__cpu_setup			// initialise processor
> > > > > > > > > >    	adrp	x1, swapper_pg_dir
> > > > > > > > > >    	bl	__enable_mmu
> > > > > > > > > > @@ -785,6 +786,31 @@ ENTRY(__enable_mmu)
> > > > > > > > > >    	ret
> > > > > > > > > >    ENDPROC(__enable_mmu)
> > > > > > > > > > +ENTRY(__cpu_secondary_check52bitva)
> > > > > > > > > > +#ifdef CONFIG_ARM64_52BIT_VA
> > > > > > > > > > +	ldr_l	x0, vabits_user
> > > > > > > > > > +	cmp	x0, #52
> > > > > > > > > > +	b.ne	2f > +
> > > > > > > > > > +	mrs_s	x0, SYS_ID_AA64MMFR2_EL1
> > > > > > > > > > +	and	x0, x0, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
> > > > > > > > > > +	cbnz	x0, 2f
> > > > > > > > > > +
> > > > > > > > > > +	adr_l	x0, va52mismatch
> > > > > > > > > > +	mov	w1, #1
> > > > > > > > > > +	strb	w1, [x0]
> > > > > > > > > > +	dmb	sy
> > > > > > > > > > +	dc	ivac, x0	// Invalidate potentially stale cache line
> > > > > > > > > 
> > > > > > > > > You may have to clear this variable before a CPU is brought up to avoid
> > > > > > > > > raising a false error message when another secondary CPU doesn't boot
> > > > > > > > > for some other reason (say granule support) after a CPU failed with lack
> > > > > > > > > of 52bitva. It is really a crazy corner case.
> > > > > > > > 
> > > > > > > > Can't we just follow the example set by the EL2 setup in the way that is
> > > > > > > > uses __boot_cpu_mode? In that case, we only need one variable and you can
> > > > > > > > detect a problem by comparing the two halves.
> > > > > > > 
> > > > > > > The only difference here is, the support is bolted at boot CPU time and hence
> > > > > > > we need to verify each and every CPU, unlike the __boot_cpu_mode where we
> > > > > > > check for mismatch after the SMP CPUs are brought up. If we decide to make
> > > > > > > the choice later, something like that could work. The only caveat is the 52bit
> > > > > > > kernel VA will have to do something like the above.
> > > > > > 
> > > > > > So looking at this a bit more, I think we're better off repurposing the
> > > > > > upper bits of the early boot status word to contain a reason code, rather
> > > > > > than introducing new variables for every possible mismatch.
> > > > > > 
> > > > > > Does the untested diff below look remotely sane to you?
> > > > > > 
> > > > > > Will
> > > > > > 
> > > > > 
> > > > > Thanks Will,
> > > > > This looks good to me, I will test now and fold this into a patch.
> > > > 
> > > > Cheers, Steve. Testing would be handy, but don't worry about respinning the
> > > > patches as I'm already on top of this and hope to push this out later today.
> > > > 
> > > 
> > > Thanks Will,
> > > This looks good to me so FWIW:
> > > Tested-by: Steve Capper <steve.capper@arm.com>
> > > 
> > > (for both the 52-bit VA mismatch and 64KB granule not supported cases
> > > using the model).
> > > 
> > > The only small issue I see is that if subsequent CPUs aren't brought
> > > online (because they don't exist in the model) then the error reason is
> > > repeated.
> > > 
> > > I'll dig into this.
> > >
> > 
> > I think __early_cpu_boot_status needs to be reset at the beginning of
> > __cpu_up before the secondary is booted. Testing a check for this now.
> >
> 




> Hi Will,
> 
> The following fixed the repeating error message problem for me. If you
> want, I can send a separate patch to fix this?
> 
> Cheers,
> -- 
> Steve
> 
> 
> --->8
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 4e3bfbde829a..936156a7ae88 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -123,6 +123,11 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  	update_cpu_boot_status(CPU_MMU_OFF);
>  	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
>  
> +	__early_cpu_boot_status = 0;
> +	dsb(ishst);
> +	__flush_dcache_area(&__early_cpu_boot_status,
> +			sizeof(__early_cpu_boot_status));
> +
>  	/*
>  	 * Now bring the CPU into our world.
>  	 */
>

I have tested Will's changes and hit the issue reported by Steve. But this
is mainly due to a bug in our __cpu_up() code, which ignores the errors reported
by the firmware and goes ahead assuming that the CPU entered the kernel and
failed so in the process.

e.g, with a missing CPU3:

[   78.050880] psci: failed to boot CPU3 (-22)
[   78.051079] CPU3: failed to boot: -22
[   78.051319] CPU3: is stuck in kernel
[   78.051496] CPU3: does not support 52-bit VAs

With the fix attached below, I get:
# echo 1 > cpu5/online
[  101.883862] CPU5: failed to come online
[  101.884860] CPU5: is stuck in kernel
[  101.885060] CPU5: does not support 52-bit VAs
-sh: echo: write error: Input/output error
# echo 1 > cpu3/online
[  106.746141] psci: failed to boot CPU3 (-22)
[  106.746360] CPU3: failed to boot: -22
-sh: echo: write error: Invalid argument


----8>----

arm64: smp: Handle errors reported by the firmware

The __cpu_up() routine ignores the errors reported by the firmware
for a CPU bringup operation and looks for the error status set by the
booting CPU. If the CPU never entered the kernel, we could end up
in assuming stale error status, which otherwise would have been
set/cleared appropriately by the booting CPU.

Reported-by: Steve Capper <steve.capper@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/smp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index ac73d6c..a854e3d 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -141,6 +141,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		}
 	} else {
 		pr_err("CPU%u: failed to boot: %d\n", cpu, ret);
+		return ret;
 	}
 
 	secondary_data.task = NULL;
-- 
2.7.4

 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 0/7] 52-bit userspace VAs

[Will Deacon]

On Thu, Dec 06, 2018 at 10:50:35PM +0000, Steve Capper wrote:
> This patch series brings support for 52-bit userspace VAs to systems that
> have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
> PAGE_SIZE.
> 
> If no hardware support is present, the kernel runs with a 48-bit VA space
> for userspace.
> 
> Userspace can exploit this feature by providing an address hint to mmap
> where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
> same way as a 48-bit VA system (this is to maintain compatibility with
> software that assumes the maximum VA size on arm64 is 48-bit).
> 
> This patch series applies to 4.20-rc1.
> 
> Testing was in a model with Trusted Firmware and UEFI for boot.
> 
> Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
> check for VA space support mismatch between CPUs.

I was all ready to push this out, but I spotted a build failure with
allmodconfig because TASK_SIZE refers to the non-EXPORTed symbol
vabits_user:

ERROR: "vabits_user" [lib/test_user_copy.ko] undefined!
ERROR: "vabits_user" [drivers/misc/lkdtm/lkdtm.ko] undefined!
ERROR: "vabits_user" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!

So I've pushed an extra patch on top to fix that by exporting the symbol.

Will

Re: [PATCH V5 0/7] 52-bit userspace VAs

[Will Deacon]

On Thu, Dec 06, 2018 at 10:50:35PM +0000, Steve Capper wrote:
> This patch series brings support for 52-bit userspace VAs to systems that
> have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
> PAGE_SIZE.
> 
> If no hardware support is present, the kernel runs with a 48-bit VA space
> for userspace.
> 
> Userspace can exploit this feature by providing an address hint to mmap
> where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
> same way as a 48-bit VA system (this is to maintain compatibility with
> software that assumes the maximum VA size on arm64 is 48-bit).
> 
> This patch series applies to 4.20-rc1.
> 
> Testing was in a model with Trusted Firmware and UEFI for boot.
> 
> Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
> check for VA space support mismatch between CPUs.

I was all ready to push this out, but I spotted a build failure with
allmodconfig because TASK_SIZE refers to the non-EXPORTed symbol
vabits_user:

ERROR: "vabits_user" [lib/test_user_copy.ko] undefined!
ERROR: "vabits_user" [drivers/misc/lkdtm/lkdtm.ko] undefined!
ERROR: "vabits_user" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!

So I've pushed an extra patch on top to fix that by exporting the symbol.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 0/7] 52-bit userspace VAs

[Steve Capper]

Hi Will,

On Mon, Dec 10, 2018 at 07:34:46PM +0000, Will Deacon wrote:
> On Thu, Dec 06, 2018 at 10:50:35PM +0000, Steve Capper wrote:
> > This patch series brings support for 52-bit userspace VAs to systems that
> > have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
> > PAGE_SIZE.
> > 
> > If no hardware support is present, the kernel runs with a 48-bit VA space
> > for userspace.
> > 
> > Userspace can exploit this feature by providing an address hint to mmap
> > where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
> > same way as a 48-bit VA system (this is to maintain compatibility with
> > software that assumes the maximum VA size on arm64 is 48-bit).
> > 
> > This patch series applies to 4.20-rc1.
> > 
> > Testing was in a model with Trusted Firmware and UEFI for boot.
> > 
> > Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
> > check for VA space support mismatch between CPUs.
> 
> I was all ready to push this out, but I spotted a build failure with
> allmodconfig because TASK_SIZE refers to the non-EXPORTed symbol
> vabits_user:
> 
> ERROR: "vabits_user" [lib/test_user_copy.ko] undefined!
> ERROR: "vabits_user" [drivers/misc/lkdtm/lkdtm.ko] undefined!
> ERROR: "vabits_user" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!

Apologies for that, I'll be more careful with modules in future.

> 
> So I've pushed an extra patch on top to fix that by exporting the symbol.
> 

Thanks!

Cheers,
-- 
Steve

Re: [PATCH V5 0/7] 52-bit userspace VAs

[Steve Capper]

Hi Will,

On Mon, Dec 10, 2018 at 07:34:46PM +0000, Will Deacon wrote:
> On Thu, Dec 06, 2018 at 10:50:35PM +0000, Steve Capper wrote:
> > This patch series brings support for 52-bit userspace VAs to systems that
> > have ARMv8.2-LVA and are running with a 48-bit VA_BITS and a 64KB
> > PAGE_SIZE.
> > 
> > If no hardware support is present, the kernel runs with a 48-bit VA space
> > for userspace.
> > 
> > Userspace can exploit this feature by providing an address hint to mmap
> > where addr[51:48] != 0. Otherwise all the VA mappings will behave in the
> > same way as a 48-bit VA system (this is to maintain compatibility with
> > software that assumes the maximum VA size on arm64 is 48-bit).
> > 
> > This patch series applies to 4.20-rc1.
> > 
> > Testing was in a model with Trusted Firmware and UEFI for boot.
> > 
> > Changed in V5, ttbr1 offsetting code simplified. Extra patch added to
> > check for VA space support mismatch between CPUs.
> 
> I was all ready to push this out, but I spotted a build failure with
> allmodconfig because TASK_SIZE refers to the non-EXPORTed symbol
> vabits_user:
> 
> ERROR: "vabits_user" [lib/test_user_copy.ko] undefined!
> ERROR: "vabits_user" [drivers/misc/lkdtm/lkdtm.ko] undefined!
> ERROR: "vabits_user" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!

Apologies for that, I'll be more careful with modules in future.

> 
> So I've pushed an extra patch on top to fix that by exporting the symbol.
> 

Thanks!

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel