Convert the remaining uaccess_* calls from ASM macros to C inlines.
These patches apply against linux-next. I boot tested ARM64, and
compile tested ARM changes.
Pavel Tatashin (3):
arm/arm64/xen: use C inlines for privcmd_call
arm64: remove uaccess_ttbr0 asm macros from cache functions
arm64: remove the rest of asm-uaccess.h
arch/arm/include/asm/assembler.h | 2 +-
arch/arm/include/asm/uaccess.h | 32 ++++++++++++---
arch/arm/xen/enlighten.c | 2 +-
arch/arm/xen/hypercall.S | 15 +------
arch/arm64/include/asm/asm-uaccess.h | 60 ----------------------------
arch/arm64/include/asm/cacheflush.h | 38 ++++++++++++++++--
arch/arm64/kernel/entry.S | 6 +--
arch/arm64/lib/clear_user.S | 2 +-
arch/arm64/lib/copy_from_user.S | 2 +-
arch/arm64/lib/copy_in_user.S | 2 +-
arch/arm64/lib/copy_to_user.S | 2 +-
arch/arm64/mm/cache.S | 31 +++++---------
arch/arm64/mm/context.c | 12 ++++++
arch/arm64/mm/flush.c | 2 +-
arch/arm64/xen/hypercall.S | 19 +--------
arch/xtensa/kernel/coprocessor.S | 1 -
include/xen/arm/hypercall.h | 23 +++++++++--
17 files changed, 117 insertions(+), 134 deletions(-)
delete mode 100644 arch/arm64/include/asm/asm-uaccess.h
--
2.24.0
privcmd_call requires to enable access to userspace for the
duration of the hypercall.
Currently, this is done via assembly macros. Change it to C
inlines instead.
Signed-off-by: Pavel Tatashin <[email protected]>
---
arch/arm/include/asm/assembler.h | 2 +-
arch/arm/include/asm/uaccess.h | 32 ++++++++++++++++++++++++++------
arch/arm/xen/enlighten.c | 2 +-
arch/arm/xen/hypercall.S | 15 ++-------------
arch/arm64/xen/hypercall.S | 19 ++-----------------
include/xen/arm/hypercall.h | 23 ++++++++++++++++++++---
6 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 99929122dad7..8e9262a0f016 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -480,7 +480,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT )
.macro uaccess_disable, tmp, isb=1
#ifdef CONFIG_CPU_SW_DOMAIN_PAN
/*
- * Whenever we re-enter userspace, the domains should always be
+ * Whenever we re-enter kernel, the domains should always be
* set appropriately.
*/
mov \tmp, #DACR_UACCESS_DISABLE
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 98c6b91be4a8..79d4efa3eb62 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -16,6 +16,23 @@
#include <asm/extable.h>
+#ifdef CONFIG_CPU_SW_DOMAIN_PAN
+static __always_inline void uaccess_enable(void)
+{
+ unsigned long val = DACR_UACCESS_ENABLE;
+
+ asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
+ isb();
+}
+
+static __always_inline void uaccess_disable(void)
+{
+ unsigned long val = DACR_UACCESS_ENABLE;
+
+ asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
+ isb();
+}
+
/*
* These two functions allow hooking accesses to userspace to increase
* system integrity by ensuring that the kernel can not inadvertantly
@@ -24,7 +41,6 @@
*/
static __always_inline unsigned int uaccess_save_and_enable(void)
{
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
unsigned int old_domain = get_domain();
/* Set the current domain access to permit user accesses */
@@ -32,18 +48,22 @@ static __always_inline unsigned int uaccess_save_and_enable(void)
domain_val(DOMAIN_USER, DOMAIN_CLIENT));
return old_domain;
-#else
- return 0;
-#endif
}
static __always_inline void uaccess_restore(unsigned int flags)
{
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
/* Restore the user access mask */
set_domain(flags);
-#endif
}
+#else
+static __always_inline void uaccess_enable(void) {}
+static __always_inline void uaccess_disable(void) {}
+static __always_inline unsigned int uaccess_save_and_enable(void)
+{
+ return 0;
+}
+static __always_inline void uaccess_restore(unsigned int flags) {}
+#endif /* CONFIG_CPU_SW_DOMAIN_PAN */
/*
* These two are intentionally not defined anywhere - if the kernel
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index dd6804a64f1a..e87280c6d25d 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -440,4 +440,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_platform_op_raw);
EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
EXPORT_SYMBOL_GPL(HYPERVISOR_vm_assist);
EXPORT_SYMBOL_GPL(HYPERVISOR_dm_op);
-EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(arch_privcmd_call);
diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
index b11bba542fac..2f5be0dc6195 100644
--- a/arch/arm/xen/hypercall.S
+++ b/arch/arm/xen/hypercall.S
@@ -94,29 +94,18 @@ HYPERCALL2(multicall);
HYPERCALL2(vm_assist);
HYPERCALL3(dm_op);
-ENTRY(privcmd_call)
+ENTRY(arch_privcmd_call)
stmdb sp!, {r4}
mov r12, r0
mov r0, r1
mov r1, r2
mov r2, r3
ldr r3, [sp, #8]
- /*
- * Privcmd calls are issued by the userspace. We need to allow the
- * kernel to access the userspace memory before issuing the hypercall.
- */
- uaccess_enable r4
/* r4 is loaded now as we use it as scratch register before */
ldr r4, [sp, #4]
__HVC(XEN_IMM)
- /*
- * Disable userspace access from kernel. This is fine to do it
- * unconditionally as no set_fs(KERNEL_DS) is called before.
- */
- uaccess_disable r4
-
ldm sp!, {r4}
ret lr
-ENDPROC(privcmd_call);
+ENDPROC(arch_privcmd_call);
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index c5f05c4a4d00..921611778d2a 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -49,7 +49,6 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
-#include <asm/asm-uaccess.h>
#include <xen/interface/xen.h>
@@ -86,27 +85,13 @@ HYPERCALL2(multicall);
HYPERCALL2(vm_assist);
HYPERCALL3(dm_op);
-ENTRY(privcmd_call)
+ENTRY(arch_privcmd_call)
mov x16, x0
mov x0, x1
mov x1, x2
mov x2, x3
mov x3, x4
mov x4, x5
- /*
- * Privcmd calls are issued by the userspace. The kernel needs to
- * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
- * translations to user memory via AT instructions. Since AT
- * instructions are not affected by the PAN bit (ARMv8.1), we only
- * need the explicit uaccess_enable/disable if the TTBR0 PAN emulation
- * is enabled (it implies that hardware UAO and PAN disabled).
- */
- uaccess_ttbr0_enable x6, x7, x8
hvc XEN_IMM
-
- /*
- * Disable userspace access from kernel once the hyp call completed.
- */
- uaccess_ttbr0_disable x6, x7
ret
-ENDPROC(privcmd_call);
+ENDPROC(arch_privcmd_call);
diff --git a/include/xen/arm/hypercall.h b/include/xen/arm/hypercall.h
index b40485e54d80..cfb704fd78c8 100644
--- a/include/xen/arm/hypercall.h
+++ b/include/xen/arm/hypercall.h
@@ -34,16 +34,33 @@
#define _ASM_ARM_XEN_HYPERCALL_H
#include <linux/bug.h>
+#include <linux/uaccess.h>
#include <xen/interface/xen.h>
#include <xen/interface/sched.h>
#include <xen/interface/platform.h>
struct xen_dm_op_buf;
+long arch_privcmd_call(unsigned int call, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ unsigned long a4, unsigned long a5);
-long privcmd_call(unsigned call, unsigned long a1,
- unsigned long a2, unsigned long a3,
- unsigned long a4, unsigned long a5);
+static inline long privcmd_call(unsigned int call, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ unsigned long a4, unsigned long a5)
+{
+ long rv;
+
+ /*
+ * Privcmd calls are issued by the userspace. We need to allow the
+ * kernel to access the userspace memory before issuing the hypercall.
+ */
+ uaccess_enable();
+ rv = arch_privcmd_call(call, a1, a2, a3, a4, a5);
+ uaccess_disable();
+
+ return rv;
+}
int HYPERVISOR_xen_version(int cmd, void *arg);
int HYPERVISOR_console_io(int cmd, int count, char *str);
int HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count);
--
2.24.0
The __uaccess_ttbr0_disable and __uaccess_ttbr0_enable,
are the last two macros defined in asm-uaccess.h.
Replace them with C wrappers and call C functions from
kernel_entry and kernel_exit.
Signed-off-by: Pavel Tatashin <[email protected]>
---
arch/arm64/include/asm/asm-uaccess.h | 38 ----------------------------
arch/arm64/kernel/entry.S | 6 ++---
arch/arm64/lib/clear_user.S | 2 +-
arch/arm64/lib/copy_from_user.S | 2 +-
arch/arm64/lib/copy_in_user.S | 2 +-
arch/arm64/lib/copy_to_user.S | 2 +-
arch/arm64/mm/cache.S | 1 -
arch/arm64/mm/context.c | 12 +++++++++
arch/xtensa/kernel/coprocessor.S | 1 -
9 files changed, 19 insertions(+), 47 deletions(-)
delete mode 100644 arch/arm64/include/asm/asm-uaccess.h
diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
deleted file mode 100644
index 8f763e5b41b1..000000000000
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __ASM_ASM_UACCESS_H
-#define __ASM_ASM_UACCESS_H
-
-#include <asm/alternative.h>
-#include <asm/kernel-pgtable.h>
-#include <asm/mmu.h>
-#include <asm/sysreg.h>
-#include <asm/assembler.h>
-
-/*
- * User access enabling/disabling macros.
- */
-#ifdef CONFIG_ARM64_SW_TTBR0_PAN
- .macro __uaccess_ttbr0_disable, tmp1
- mrs \tmp1, ttbr1_el1 // swapper_pg_dir
- bic \tmp1, \tmp1, #TTBR_ASID_MASK
- sub \tmp1, \tmp1, #RESERVED_TTBR0_SIZE // reserved_ttbr0 just before swapper_pg_dir
- msr ttbr0_el1, \tmp1 // set reserved TTBR0_EL1
- isb
- add \tmp1, \tmp1, #RESERVED_TTBR0_SIZE
- msr ttbr1_el1, \tmp1 // set reserved ASID
- isb
- .endm
-
- .macro __uaccess_ttbr0_enable, tmp1, tmp2
- get_current_task \tmp1
- ldr \tmp1, [\tmp1, #TSK_TI_TTBR0] // load saved TTBR0_EL1
- mrs \tmp2, ttbr1_el1
- extr \tmp2, \tmp2, \tmp1, #48
- ror \tmp2, \tmp2, #16
- msr ttbr1_el1, \tmp2 // set the active ASID
- isb
- msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
- isb
- .endm
-#endif
-#endif
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 583f71abbe98..c7b571e6d0f2 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -22,8 +22,8 @@
#include <asm/mmu.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
+#include <asm/kernel-pgtable.h>
#include <asm/thread_info.h>
-#include <asm/asm-uaccess.h>
#include <asm/unistd.h>
/*
@@ -219,7 +219,7 @@ alternative_else_nop_endif
and x23, x23, #~PSR_PAN_BIT // Clear the emulated PAN in the saved SPSR
.endif
- __uaccess_ttbr0_disable x21
+ bl __uaccess_ttbr0_disable_c
1:
#endif
@@ -293,7 +293,7 @@ alternative_else_nop_endif
tbnz x22, #22, 1f // Skip re-enabling TTBR0 access if the PSR_PAN_BIT is set
.endif
- __uaccess_ttbr0_enable x0, x1
+ bl __uaccess_ttbr0_enable_c
.if \el == 0
/*
diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index aeafc03e961a..b0b4a86a09e2 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -6,7 +6,7 @@
*/
#include <linux/linkage.h>
-#include <asm/asm-uaccess.h>
+#include <asm/alternative.h>
#include <asm/assembler.h>
.text
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index ebb3c06cbb5d..142bc7505518 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -5,7 +5,7 @@
#include <linux/linkage.h>
-#include <asm/asm-uaccess.h>
+#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
index 3d8153a1ebce..04dc48ca26f7 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -7,7 +7,7 @@
#include <linux/linkage.h>
-#include <asm/asm-uaccess.h>
+#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index 357eae2c18eb..8f3218ae88ab 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -5,7 +5,7 @@
#include <linux/linkage.h>
-#include <asm/asm-uaccess.h>
+#include <asm/alternative.h>
#include <asm/assembler.h>
#include <asm/cache.h>
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 408d317a47d2..7940d6ef5da5 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -12,7 +12,6 @@
#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/alternative.h>
-#include <asm/asm-uaccess.h>
/*
* __arch_flush_icache_range(start,end)
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index b5e329fde2dd..4fc32c504dea 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -237,6 +237,18 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu)
cpu_switch_mm(mm->pgd, mm);
}
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+asmlinkage void __uaccess_ttbr0_enable_c(void)
+{
+ __uaccess_ttbr0_enable();
+}
+
+asmlinkage void __uaccess_ttbr0_disable_c(void)
+{
+ __uaccess_ttbr0_disable();
+}
+#endif
+
/* Errata workaround post TTBRx_EL1 update. */
asmlinkage void post_ttbr_update_workaround(void)
{
diff --git a/arch/xtensa/kernel/coprocessor.S b/arch/xtensa/kernel/coprocessor.S
index 80828b95a51f..6329d17e2aa0 100644
--- a/arch/xtensa/kernel/coprocessor.S
+++ b/arch/xtensa/kernel/coprocessor.S
@@ -18,7 +18,6 @@
#include <asm/processor.h>
#include <asm/coprocessor.h>
#include <asm/thread_info.h>
-#include <asm/asm-uaccess.h>
#include <asm/unistd.h>
#include <asm/ptrace.h>
#include <asm/current.h>
--
2.24.0
Replace the uaccess_ttbr0_disable/uaccess_ttbr0_enable via
inline variants, and remove asm macros.
Signed-off-by: Pavel Tatashin <[email protected]>
---
arch/arm64/include/asm/asm-uaccess.h | 22 ----------------
arch/arm64/include/asm/cacheflush.h | 38 +++++++++++++++++++++++++---
arch/arm64/mm/cache.S | 30 ++++++++--------------
arch/arm64/mm/flush.c | 2 +-
4 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
index 35e6145e1402..8f763e5b41b1 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -34,27 +34,5 @@
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..cdd4a8eb8708 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -61,16 +61,48 @@
* - 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 __arch_flush_icache_range(unsigned long start, unsigned long end);
+extern long __arch_flush_cache_user_range(unsigned long start,
+ unsigned long end);
+extern int arch_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_icache_range(unsigned long start, unsigned long end)
+{
+ uaccess_ttbr0_enable();
+ __arch_flush_icache_range(start, end);
+ uaccess_ttbr0_disable();
+}
+
+static inline void __flush_cache_user_range(unsigned long start,
+ unsigned long end)
+{
+ uaccess_ttbr0_enable();
+ __arch_flush_cache_user_range(start, end);
+ uaccess_ttbr0_disable();
+}
+
+static inline int invalidate_icache_range(unsigned long start,
+ unsigned long end)
+{
+ int rv;
+#if ARM64_HAS_CACHE_DIC
+ rv = arch_invalidate_icache_range(start, end);
+#else
+ uaccess_ttbr0_enable();
+ rv = arch_invalidate_icache_range(start, end);
+ uaccess_ttbr0_disable();
+#endif
+ return rv;
+}
+
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..408d317a47d2 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)
+ * __arch_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(__arch_flush_icache_range)
/* FALLTHROUGH */
/*
- * __flush_cache_user_range(start,end)
+ * __arch_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(__arch_flush_cache_user_range)
alternative_if ARM64_HAS_CACHE_IDC
dsb ishst
b 7f
@@ -60,14 +59,11 @@ 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(__arch_flush_icache_range)
+ENDPROC(__arch_flush_cache_user_range)
/*
* invalidate_icache_range(start,end)
@@ -83,16 +79,10 @@ alternative_if ARM64_HAS_CACHE_DIC
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)
diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
index ac485163a4a7..66249fca2092 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(__arch_flush_icache_range);
#ifdef CONFIG_ARCH_HAS_PMEM_API
void arch_wb_cache_pmem(void *addr, size_t size)
--
2.24.0
On Thu, Nov 21, 2019 at 01:48:03PM -0500, Pavel Tatashin wrote:
> privcmd_call requires to enable access to userspace for the
> duration of the hypercall.
>
> Currently, this is done via assembly macros. Change it to C
> inlines instead.
>
> Signed-off-by: Pavel Tatashin <[email protected]>
> ---
> arch/arm/include/asm/assembler.h | 2 +-
> arch/arm/include/asm/uaccess.h | 32 ++++++++++++++++++++++++++------
> arch/arm/xen/enlighten.c | 2 +-
> arch/arm/xen/hypercall.S | 15 ++-------------
> arch/arm64/xen/hypercall.S | 19 ++-----------------
> include/xen/arm/hypercall.h | 23 ++++++++++++++++++++---
> 6 files changed, 52 insertions(+), 41 deletions(-)
>
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 99929122dad7..8e9262a0f016 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -480,7 +480,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT )
> .macro uaccess_disable, tmp, isb=1
> #ifdef CONFIG_CPU_SW_DOMAIN_PAN
> /*
> - * Whenever we re-enter userspace, the domains should always be
> + * Whenever we re-enter kernel, the domains should always be
> * set appropriately.
> */
> mov \tmp, #DACR_UACCESS_DISABLE
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index 98c6b91be4a8..79d4efa3eb62 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -16,6 +16,23 @@
>
> #include <asm/extable.h>
>
> +#ifdef CONFIG_CPU_SW_DOMAIN_PAN
> +static __always_inline void uaccess_enable(void)
> +{
> + unsigned long val = DACR_UACCESS_ENABLE;
> +
> + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> + isb();
> +}
> +
> +static __always_inline void uaccess_disable(void)
> +{
> + unsigned long val = DACR_UACCESS_ENABLE;
> +
> + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> + isb();
> +}
Rather than inventing these, why not use uaccess_save_and_enable()..
uaccess_restore() around the Xen call?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
> > +#ifdef CONFIG_CPU_SW_DOMAIN_PAN
> > +static __always_inline void uaccess_enable(void)
> > +{
> > + unsigned long val = DACR_UACCESS_ENABLE;
> > +
> > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > + isb();
> > +}
> > +
> > +static __always_inline void uaccess_disable(void)
> > +{
> > + unsigned long val = DACR_UACCESS_ENABLE;
Oops, should be DACR_UACCESS_DISABLE.
> > +
> > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > + isb();
> > +}
>
> Rather than inventing these, why not use uaccess_save_and_enable()..
> uaccess_restore() around the Xen call?
Thank you for suggestion: uaccess_enable() and uaccess_disable() are
common calls with arm64, so I will need them, but I think I can use
set_domain() with DACR_UACCESS_DISABLE /DACR_UACCESS_ENABLE inside
these inlines.
Pasha
On Thu, Nov 21, 2019 at 07:30:41PM -0500, Pavel Tatashin wrote:
> > > +#ifdef CONFIG_CPU_SW_DOMAIN_PAN
> > > +static __always_inline void uaccess_enable(void)
> > > +{
> > > + unsigned long val = DACR_UACCESS_ENABLE;
> > > +
> > > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > > + isb();
> > > +}
> > > +
> > > +static __always_inline void uaccess_disable(void)
> > > +{
> > > + unsigned long val = DACR_UACCESS_ENABLE;
>
> Oops, should be DACR_UACCESS_DISABLE.
>
> > > +
> > > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > > + isb();
> > > +}
> >
> > Rather than inventing these, why not use uaccess_save_and_enable()..
> > uaccess_restore() around the Xen call?
>
> Thank you for suggestion: uaccess_enable() and uaccess_disable() are
> common calls with arm64, so I will need them, but I think I can use
> set_domain() with DACR_UACCESS_DISABLE /DACR_UACCESS_ENABLE inside
> these inlines.
That may be, but be very careful that you only use them in ARMv7-only
code. Using them elsewhere is unsafe as the domain register is used
for other purposes, and merely blatting over it (as your
uaccess_enable and uaccess_disable functions do) is unsafe.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Fri, Nov 22, 2019 at 12:34:03AM +0000, Russell King - ARM Linux admin wrote:
> On Thu, Nov 21, 2019 at 07:30:41PM -0500, Pavel Tatashin wrote:
> > > > +#ifdef CONFIG_CPU_SW_DOMAIN_PAN
> > > > +static __always_inline void uaccess_enable(void)
> > > > +{
> > > > + unsigned long val = DACR_UACCESS_ENABLE;
> > > > +
> > > > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > > > + isb();
> > > > +}
> > > > +
> > > > +static __always_inline void uaccess_disable(void)
> > > > +{
> > > > + unsigned long val = DACR_UACCESS_ENABLE;
> >
> > Oops, should be DACR_UACCESS_DISABLE.
> >
> > > > +
> > > > + asm volatile("mcr p15, 0, %0, c3, c0, 0" : : "r" (val));
> > > > + isb();
> > > > +}
> > >
> > > Rather than inventing these, why not use uaccess_save_and_enable()..
> > > uaccess_restore() around the Xen call?
> >
> > Thank you for suggestion: uaccess_enable() and uaccess_disable() are
> > common calls with arm64, so I will need them, but I think I can use
> > set_domain() with DACR_UACCESS_DISABLE /DACR_UACCESS_ENABLE inside
> > these inlines.
>
> That may be, but be very careful that you only use them in ARMv7-only
> code. Using them elsewhere is unsafe as the domain register is used
> for other purposes, and merely blatting over it (as your
> uaccess_enable and uaccess_disable functions do) is unsafe.
In fact, I'll turn that into a bit more than a suggestion. I'll make
it a NAK on adding them to 32-bit ARM.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
> > That may be, but be very careful that you only use them in ARMv7-only
> > code. Using them elsewhere is unsafe as the domain register is used
> > for other purposes, and merely blatting over it (as your
> > uaccess_enable and uaccess_disable functions do) is unsafe.
>
> In fact, I'll turn that into a bit more than a suggestion. I'll make
> it a NAK on adding them to 32-bit ARM.
>
That's fine, and I also did not want to change ARM 32-bit. But, do you
have a suggestion how differentiate between arm64 and arm in
include/xen/arm/hypercall.h without ugly ifdefs?
Thank you,
Pasha
On Thu, Nov 21, 2019 at 07:39:22PM -0500, Pavel Tatashin wrote:
> > > That may be, but be very careful that you only use them in ARMv7-only
> > > code. Using them elsewhere is unsafe as the domain register is used
> > > for other purposes, and merely blatting over it (as your
> > > uaccess_enable and uaccess_disable functions do) is unsafe.
> >
> > In fact, I'll turn that into a bit more than a suggestion. I'll make
> > it a NAK on adding them to 32-bit ARM.
> >
>
> That's fine, and I also did not want to change ARM 32-bit. But, do you
> have a suggestion how differentiate between arm64 and arm in
> include/xen/arm/hypercall.h without ugly ifdefs?
Sorry, I don't.
I'm surprised ARM64 doesn't have anything like that, but I suspect
that's because they don't need to do a save/restore type operation.
Whereas, 32-bit ARM does very much need the save/restore behaviour
(although not in this path.)
The problem is, turning uaccess_enable/disable into C code means
that it's open to being used elsewhere in the kernel (ooh, a couple
of useful looking functions that work on both architectures! I can
use that too!) and then we end up with stuff breaking subtly. It's
the potential for subtle breakage that is making me NAK the idea of
adding the inline C functions.
Given the two have diverged, the only answer is ifdefs, sorry.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Thu, Nov 21, 2019 at 10:50 AM Pavel Tatashin
<[email protected]> wrote:
>
> The __uaccess_ttbr0_disable and __uaccess_ttbr0_enable,
> are the last two macros defined in asm-uaccess.h.
>
> Replace them with C wrappers and call C functions from
> kernel_entry and kernel_exit.
>
> Signed-off-by: Pavel Tatashin <[email protected]>
> ---
> arch/arm64/include/asm/asm-uaccess.h | 38 ----------------------------
> arch/arm64/kernel/entry.S | 6 ++---
> arch/arm64/lib/clear_user.S | 2 +-
> arch/arm64/lib/copy_from_user.S | 2 +-
> arch/arm64/lib/copy_in_user.S | 2 +-
> arch/arm64/lib/copy_to_user.S | 2 +-
> arch/arm64/mm/cache.S | 1 -
> arch/arm64/mm/context.c | 12 +++++++++
> arch/xtensa/kernel/coprocessor.S | 1 -
> 9 files changed, 19 insertions(+), 47 deletions(-)
> delete mode 100644 arch/arm64/include/asm/asm-uaccess.h
[...]
> diff --git a/arch/xtensa/kernel/coprocessor.S b/arch/xtensa/kernel/coprocessor.S
> index 80828b95a51f..6329d17e2aa0 100644
> --- a/arch/xtensa/kernel/coprocessor.S
> +++ b/arch/xtensa/kernel/coprocessor.S
> @@ -18,7 +18,6 @@
> #include <asm/processor.h>
> #include <asm/coprocessor.h>
> #include <asm/thread_info.h>
> -#include <asm/asm-uaccess.h>
> #include <asm/unistd.h>
> #include <asm/ptrace.h>
> #include <asm/current.h>
This is not related to arm64 or to the changes in the description,
but the change itself is OK. Whether you keep it in this patch,
or choose to split it out feel free to add
Acked-by: Max Filippov <[email protected]> # for xtensa bits
--
Thanks.
-- Max
> This is not related to arm64 or to the changes in the description,
> but the change itself is OK. Whether you keep it in this patch,
> or choose to split it out feel free to add
>
> Acked-by: Max Filippov <[email protected]> # for xtensa bits
Sorry, this was accidental change. I will remove it from the next
version of this series.
Pasha