[v4,3/6] arm64: remove uaccess_ttbr0 asm macros from cache functions
diff mbox series

Message ID 20191204232058.2500117-4-pasha.tatashin@soleen.com
State New
Headers show
Series
  • Use C inlines for uaccess
Related show

Commit Message

Pavel Tatashin Dec. 4, 2019, 11:20 p.m. UTC
We currently duplicate the logic to enable/disable uaccess via TTBR0,
with C functions and assembly macros. This is a maintenenace burden
and is liable to lead to subtle bugs, so let's get rid of the assembly
macros, and always use the C functions. This requires refactoring
some assembly functions to have a C wrapper.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 arch/arm64/include/asm/asm-uaccess.h | 22 -----------------
 arch/arm64/include/asm/cacheflush.h  | 35 ++++++++++++++++++++++++---
 arch/arm64/mm/cache.S                | 36 ++++++++++------------------
 arch/arm64/mm/flush.c                |  2 +-
 4 files changed, 46 insertions(+), 49 deletions(-)

Comments

kbuild test robot Dec. 7, 2019, 8:33 p.m. UTC | #1
Hi Pavel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20191206]
[cannot apply to arm64/for-next/core xen-tip/linux-next v5.4-rc8 arm/for-next v5.4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Pavel-Tatashin/Use-C-inlines-for-uaccess/20191207-044947
base:    838333c80c4f64a4ef9f5486f8bbc73312cd3abf
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/arm64/kernel/sys_compat.c: In function '__do_compat_cache_op':
>> arch/arm64/kernel/sys_compat.c:44:7: error: void value not ignored as it ought to be
      ret = __flush_cache_user_range(start, start + chunk);
          ^

vim +44 arch/arm64/kernel/sys_compat.c

3dd681d944f6d8 Will Deacon     2012-03-05  23  
a2d25a5391ca21 Vladimir Murzin 2014-12-01  24  static long
a2d25a5391ca21 Vladimir Murzin 2014-12-01  25  __do_compat_cache_op(unsigned long start, unsigned long end)
3dd681d944f6d8 Will Deacon     2012-03-05  26  {
a2d25a5391ca21 Vladimir Murzin 2014-12-01  27  	long ret;
3dd681d944f6d8 Will Deacon     2012-03-05  28  
a2d25a5391ca21 Vladimir Murzin 2014-12-01  29  	do {
a2d25a5391ca21 Vladimir Murzin 2014-12-01  30  		unsigned long chunk = min(PAGE_SIZE, end - start);
3dd681d944f6d8 Will Deacon     2012-03-05  31  
a2d25a5391ca21 Vladimir Murzin 2014-12-01  32  		if (fatal_signal_pending(current))
a2d25a5391ca21 Vladimir Murzin 2014-12-01  33  			return 0;
a2d25a5391ca21 Vladimir Murzin 2014-12-01  34  
222fc0c8503d98 James Morse     2019-10-17  35  		if (cpus_have_const_cap(ARM64_WORKAROUND_1542419)) {
222fc0c8503d98 James Morse     2019-10-17  36  			/*
222fc0c8503d98 James Morse     2019-10-17  37  			 * The workaround requires an inner-shareable tlbi.
222fc0c8503d98 James Morse     2019-10-17  38  			 * We pick the reserved-ASID to minimise the impact.
222fc0c8503d98 James Morse     2019-10-17  39  			 */
27a22fbdeedd6c Catalin Marinas 2019-10-28  40  			__tlbi(aside1is, __TLBI_VADDR(0, 0));
222fc0c8503d98 James Morse     2019-10-17  41  			dsb(ish);
222fc0c8503d98 James Morse     2019-10-17  42  		}
222fc0c8503d98 James Morse     2019-10-17  43  
a2d25a5391ca21 Vladimir Murzin 2014-12-01 @44  		ret = __flush_cache_user_range(start, start + chunk);
a2d25a5391ca21 Vladimir Murzin 2014-12-01  45  		if (ret)
a2d25a5391ca21 Vladimir Murzin 2014-12-01  46  			return ret;
a2d25a5391ca21 Vladimir Murzin 2014-12-01  47  
a2d25a5391ca21 Vladimir Murzin 2014-12-01  48  		cond_resched();
a2d25a5391ca21 Vladimir Murzin 2014-12-01  49  		start += chunk;
a2d25a5391ca21 Vladimir Murzin 2014-12-01  50  	} while (start < end);
a2d25a5391ca21 Vladimir Murzin 2014-12-01  51  
a2d25a5391ca21 Vladimir Murzin 2014-12-01  52  	return 0;
3dd681d944f6d8 Will Deacon     2012-03-05  53  }
3dd681d944f6d8 Will Deacon     2012-03-05  54  

:::::: The code at line 44 was first introduced by commit
:::::: a2d25a5391ca219f196f9fee7b535c40d201c6bf arm64: compat: align cacheflush syscall with arch/arm

:::::: TO: Vladimir Murzin <vladimir.murzin@arm.com>
:::::: CC: Will Deacon <will.deacon@arm.com>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

Patch
diff mbox series

diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
index f68a0e64482a..fba2a69f7fef 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -34,28 +34,6 @@ 
 	msr	ttbr0_el1, \tmp1		// set the non-PAN TTBR0_EL1
 	isb
 	.endm
-
-	.macro	uaccess_ttbr0_disable, tmp1, tmp2
-alternative_if_not ARM64_HAS_PAN
-	save_and_disable_irq \tmp2		// avoid preemption
-	__uaccess_ttbr0_disable \tmp1
-	restore_irq \tmp2
-alternative_else_nop_endif
-	.endm
-
-	.macro	uaccess_ttbr0_enable, tmp1, tmp2, tmp3
-alternative_if_not ARM64_HAS_PAN
-	save_and_disable_irq \tmp3		// avoid preemption
-	__uaccess_ttbr0_enable \tmp1, \tmp2
-	restore_irq \tmp3
-alternative_else_nop_endif
-	.endm
-#else
-	.macro	uaccess_ttbr0_disable, tmp1, tmp2
-	.endm
-
-	.macro	uaccess_ttbr0_enable, tmp1, tmp2, tmp3
-	.endm
 #endif
 
 #endif
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index 665c78e0665a..431f8da2dd02 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -61,16 +61,45 @@ 
  *		- kaddr  - page address
  *		- size   - region size
  */
-extern void __flush_icache_range(unsigned long start, unsigned long end);
-extern int  invalidate_icache_range(unsigned long start, unsigned long end);
+extern void __asm_flush_icache_range(unsigned long start, unsigned long end);
+extern long __asm_flush_cache_user_range(unsigned long start,
+					 unsigned long end);
+extern int  __asm_invalidate_icache_range(unsigned long start,
+					  unsigned long end);
 extern void __flush_dcache_area(void *addr, size_t len);
 extern void __inval_dcache_area(void *addr, size_t len);
 extern void __clean_dcache_area_poc(void *addr, size_t len);
 extern void __clean_dcache_area_pop(void *addr, size_t len);
 extern void __clean_dcache_area_pou(void *addr, size_t len);
-extern long __flush_cache_user_range(unsigned long start, unsigned long end);
 extern void sync_icache_aliases(void *kaddr, unsigned long len);
 
+static inline void __flush_cache_user_range(unsigned long start,
+					    unsigned long end)
+{
+	uaccess_ttbr0_enable();
+	__asm_flush_cache_user_range(start, end);
+	uaccess_ttbr0_disable();
+}
+
+static inline void __flush_icache_range(unsigned long start, unsigned long end)
+{
+	uaccess_ttbr0_enable();
+	__asm_flush_icache_range(start, end);
+	uaccess_ttbr0_disable();
+}
+
+static inline int invalidate_icache_range(unsigned long start,
+					  unsigned long end)
+{
+	int ret;
+
+	uaccess_ttbr0_enable();
+	ret = __asm_invalidate_icache_range(start, end);
+	uaccess_ttbr0_disable();
+
+	return ret;
+}
+
 static inline void flush_icache_range(unsigned long start, unsigned long end)
 {
 	__flush_icache_range(start, end);
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index db767b072601..602b9aa8603a 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -15,7 +15,7 @@ 
 #include <asm/asm-uaccess.h>
 
 /*
- *	flush_icache_range(start,end)
+ *	__asm_flush_icache_range(start,end)
  *
  *	Ensure that the I and D caches are coherent within specified region.
  *	This is typically used when code has been written to a memory region,
@@ -24,11 +24,11 @@ 
  *	- start   - virtual start address of region
  *	- end     - virtual end address of region
  */
-ENTRY(__flush_icache_range)
+ENTRY(__asm_flush_icache_range)
 	/* FALLTHROUGH */
 
 /*
- *	__flush_cache_user_range(start,end)
+ *	__asm_flush_cache_user_range(start,end)
  *
  *	Ensure that the I and D caches are coherent within specified region.
  *	This is typically used when code has been written to a memory region,
@@ -37,8 +37,7 @@  ENTRY(__flush_icache_range)
  *	- start   - virtual start address of region
  *	- end     - virtual end address of region
  */
-ENTRY(__flush_cache_user_range)
-	uaccess_ttbr0_enable x2, x3, x4
+ENTRY(__asm_flush_cache_user_range)
 alternative_if ARM64_HAS_CACHE_IDC
 	dsb	ishst
 	b	7f
@@ -60,41 +59,32 @@  alternative_if ARM64_HAS_CACHE_DIC
 alternative_else_nop_endif
 	invalidate_icache_by_line x0, x1, x2, x3, 9f
 8:	mov	x0, #0
-1:
-	uaccess_ttbr0_disable x1, x2
-	ret
-9:
-	mov	x0, #-EFAULT
+1:	ret
+9:	mov	x0, #-EFAULT
 	b	1b
-ENDPROC(__flush_icache_range)
-ENDPROC(__flush_cache_user_range)
+ENDPROC(__asm_flush_icache_range)
+ENDPROC(__asm_flush_cache_user_range)
 
 /*
- *	invalidate_icache_range(start,end)
+ *	__asm_invalidate_icache_range(start,end)
  *
  *	Ensure that the I cache is invalid within specified region.
  *
  *	- start   - virtual start address of region
  *	- end     - virtual end address of region
  */
-ENTRY(invalidate_icache_range)
+ENTRY(__asm_invalidate_icache_range)
 alternative_if ARM64_HAS_CACHE_DIC
 	mov	x0, xzr
 	isb
 	ret
 alternative_else_nop_endif
-
-	uaccess_ttbr0_enable x2, x3, x4
-
 	invalidate_icache_by_line x0, x1, x2, x3, 2f
 	mov	x0, xzr
-1:
-	uaccess_ttbr0_disable x1, x2
-	ret
-2:
-	mov	x0, #-EFAULT
+1:	ret
+2:	mov	x0, #-EFAULT
 	b	1b
-ENDPROC(invalidate_icache_range)
+ENDPROC(__asm_invalidate_icache_range)
 
 /*
  *	__flush_dcache_area(kaddr, size)
diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
index ac485163a4a7..b23f34d23f31 100644
--- a/arch/arm64/mm/flush.c
+++ b/arch/arm64/mm/flush.c
@@ -75,7 +75,7 @@  EXPORT_SYMBOL(flush_dcache_page);
 /*
  * Additional functions defined in assembly.
  */
-EXPORT_SYMBOL(__flush_icache_range);
+EXPORT_SYMBOL(__asm_flush_icache_range);
 
 #ifdef CONFIG_ARCH_HAS_PMEM_API
 void arch_wb_cache_pmem(void *addr, size_t size)