[PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg()

[PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg()

[Mark Rutland]

A while back we added {read,write}_sysreg accessors to handle accesses
to system registers, without the usual boilerplate asm volatile,
temporary variable, etc.

These patches replace asm blocks that perform mrs/msr with
{read,write}_sysreg(), and remove code and comments that this change makes
unnecessary. Where {read,write}_sysreg() are not appropriate, comments are
added to explain why.
 
This gets rid of a reasonable amount of code, and makes what
remains easier to follow. 

Thanks,
Mark.

Mark Rutland (6):
  arm64: sysreg: allow write_sysreg to use XZR
  arm64: arch_timer: simplify accessors
  arm64: dcc: simplify accessors
  arm64/kvm: use {read,write}_sysreg()
  arm64: simplify sysreg manipulation
  arm64: simplify contextidr_thread_switch

 arch/arm64/include/asm/arch_timer.h    | 41 ++++++++++++----------------------
 arch/arm64/include/asm/dcc.h           | 14 ++++--------
 arch/arm64/include/asm/hw_breakpoint.h | 13 ++++++-----
 arch/arm64/include/asm/mmu_context.h   | 36 +++++++++++------------------
 arch/arm64/include/asm/pgtable-hwdef.h |  1 +
 arch/arm64/include/asm/sysreg.h        | 28 +++++++++++++----------
 arch/arm64/include/asm/thread_info.h   |  3 +++
 arch/arm64/include/asm/virt.h          |  6 ++---
 arch/arm64/kernel/cacheinfo.c          |  8 +++----
 arch/arm64/kernel/debug-monitors.c     |  8 +++----
 arch/arm64/kernel/process.c            | 14 +++++-------
 arch/arm64/kernel/sys_compat.c         |  2 +-
 arch/arm64/kvm/sys_regs.c              | 31 +++++++++----------------
 arch/arm64/kvm/sys_regs_generic_v8.c   |  6 ++---
 14 files changed, 86 insertions(+), 125 deletions(-)

-- 
1.9.1

[PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR

[Mark Rutland]

Currently write_sysreg has to allocate a temporary register to write
zero to a system register, which is unfortunate given that the MSR
instruction accepts XZR as an operand.

Allow XZR to be used when appropriate by fiddling with the assembly
constraints.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/sysreg.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index cc06794..39fed2e 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -273,10 +273,14 @@ static inline void config_sctlr_el1(u32 clear, u32 set)
 	__val;							\
 })
 
+/*
+ * The "Z" constraint normally means a zero immediate, but when combined with
+ * the "%x0" template means XZR.
+ */
 #define write_sysreg(v, r) do {					\
 	u64 __val = (u64)v;					\
-	asm volatile("msr " __stringify(r) ", %0"		\
-		     : : "r" (__val));				\
+	asm volatile("msr " __stringify(r) ", %x0"		\
+		     : : "rZ" (__val));				\
 } while (0)
 
 #endif
-- 
1.9.1

[PATCH 2/6] arm64: arch_timer: simplify accessors

[Mark Rutland]

A while back we added {read,write}_sysreg accessors to handle accesses
to system registers, without the usual boilerplate asm volatile,
temporary variable, etc.

This patch makes use of these in the arm64 arch timer accessors to make
the code shorter and clearer.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 41 +++++++++++++------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index fbe0ca3..7ff386c 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -20,6 +20,7 @@
 #define __ASM_ARCH_TIMER_H
 
 #include <asm/barrier.h>
+#include <asm/sysreg.h>
 
 #include <linux/bug.h>
 #include <linux/init.h>
@@ -38,19 +39,19 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u32 val)
 	if (access == ARCH_TIMER_PHYS_ACCESS) {
 		switch (reg) {
 		case ARCH_TIMER_REG_CTRL:
-			asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
+			write_sysreg(val, cntp_ctl_el0);
 			break;
 		case ARCH_TIMER_REG_TVAL:
-			asm volatile("msr cntp_tval_el0, %0" : : "r" (val));
+			write_sysreg(val, cntp_tval_el0);
 			break;
 		}
 	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
 		switch (reg) {
 		case ARCH_TIMER_REG_CTRL:
-			asm volatile("msr cntv_ctl_el0,  %0" : : "r" (val));
+			write_sysreg(val, cntv_ctl_el0);
 			break;
 		case ARCH_TIMER_REG_TVAL:
-			asm volatile("msr cntv_tval_el0, %0" : : "r" (val));
+			write_sysreg(val, cntv_tval_el0);
 			break;
 		}
 	}
@@ -61,48 +62,38 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u32 val)
 static __always_inline
 u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
 {
-	u32 val;
-
 	if (access == ARCH_TIMER_PHYS_ACCESS) {
 		switch (reg) {
 		case ARCH_TIMER_REG_CTRL:
-			asm volatile("mrs %0,  cntp_ctl_el0" : "=r" (val));
-			break;
+			return read_sysreg(cntp_ctl_el0);
 		case ARCH_TIMER_REG_TVAL:
-			asm volatile("mrs %0, cntp_tval_el0" : "=r" (val));
-			break;
+			return read_sysreg(cntp_tval_el0);
 		}
 	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
 		switch (reg) {
 		case ARCH_TIMER_REG_CTRL:
-			asm volatile("mrs %0,  cntv_ctl_el0" : "=r" (val));
-			break;
+			return read_sysreg(cntv_ctl_el0);
 		case ARCH_TIMER_REG_TVAL:
-			asm volatile("mrs %0, cntv_tval_el0" : "=r" (val));
-			break;
+			return read_sysreg(cntv_tval_el0);
 		}
 	}
 
-	return val;
+	BUG();
 }
 
 static inline u32 arch_timer_get_cntfrq(void)
 {
-	u32 val;
-	asm volatile("mrs %0,   cntfrq_el0" : "=r" (val));
-	return val;
+	return read_sysreg(cntfrq_el0);
 }
 
 static inline u32 arch_timer_get_cntkctl(void)
 {
-	u32 cntkctl;
-	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
-	return cntkctl;
+	return read_sysreg(cntkctl_el1);
 }
 
 static inline void arch_timer_set_cntkctl(u32 cntkctl)
 {
-	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
+	write_sysreg(cntkctl, cntkctl_el1);
 }
 
 static inline u64 arch_counter_get_cntpct(void)
@@ -116,12 +107,8 @@ static inline u64 arch_counter_get_cntpct(void)
 
 static inline u64 arch_counter_get_cntvct(void)
 {
-	u64 cval;
-
 	isb();
-	asm volatile("mrs %0, cntvct_el0" : "=r" (cval));
-
-	return cval;
+	return read_sysreg(cntvct_el0);
 }
 
 static inline int arch_timer_arch_init(void)
-- 
1.9.1

[PATCH 3/6] arm64: dcc: simplify accessors

[Mark Rutland]

A while back we added {read,write}_sysreg accessors to handle accesses
to system registers, without the usual boilerplate asm volatile,
temporary variable, etc.

This patch makes use of these in the arm64 DCC accessors to make the
code shorter and clearer.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/dcc.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/dcc.h b/arch/arm64/include/asm/dcc.h
index 65e0190..836b056 100644
--- a/arch/arm64/include/asm/dcc.h
+++ b/arch/arm64/include/asm/dcc.h
@@ -21,21 +21,16 @@
 #define __ASM_DCC_H
 
 #include <asm/barrier.h>
+#include <asm/sysreg.h>
 
 static inline u32 __dcc_getstatus(void)
 {
-	u32 ret;
-
-	asm volatile("mrs %0, mdccsr_el0" : "=r" (ret));
-
-	return ret;
+	return read_sysreg(mdccsr_el0);
 }
 
 static inline char __dcc_getchar(void)
 {
-	char c;
-
-	asm volatile("mrs %0, dbgdtrrx_el0" : "=r" (c));
+	char c = read_sysreg(dbgdtrrx_el0);
 	isb();
 
 	return c;
@@ -47,8 +42,7 @@ static inline void __dcc_putchar(char c)
 	 * The typecast is to make absolutely certain that 'c' is
 	 * zero-extended.
 	 */
-	asm volatile("msr dbgdtrtx_el0, %0"
-			: : "r" ((unsigned long)(unsigned char)c));
+	write_sysreg((unsigned char)c, dbgdtrtx_el0);
 	isb();
 }
 
-- 
1.9.1

[PATCH 4/6] arm64/kvm: use {read,write}_sysreg()

[Mark Rutland]

A while back we added {read,write}_sysreg accessors to handle accesses
to system registers, without the usual boilerplate asm volatile,
temporary variable, etc.

This patch makes use of these in the arm64 KVM code to make the code
shorter and clearer.

At the same time, a comment style violation next to a system register
access is fixed up in reset_pmcr, and comments describing whether
operations are reads or writes are removed as this is now painfully
obvious.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/virt.h        |  6 ++----
 arch/arm64/kvm/sys_regs.c            | 31 +++++++++++--------------------
 arch/arm64/kvm/sys_regs_generic_v8.c |  6 ++----
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 1788545..c5cc940 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -45,6 +45,7 @@
 #ifndef __ASSEMBLY__
 
 #include <asm/ptrace.h>
+#include <asm/sysreg.h>
 
 /*
  * __boot_cpu_mode records what mode CPUs were booted in.
@@ -75,10 +76,7 @@ static inline bool is_hyp_mode_mismatched(void)
 
 static inline bool is_kernel_in_hyp_mode(void)
 {
-	u64 el;
-
-	asm("mrs %0, CurrentEL" : "=r" (el));
-	return el == CurrentEL_EL2;
+	return read_sysreg(CurrentEL) == CurrentEL_EL2;
 }
 
 #ifdef CONFIG_ARM64_VHE
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e51367d..f302fdb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -36,6 +36,7 @@
 #include <asm/kvm_host.h>
 #include <asm/kvm_mmu.h>
 #include <asm/perf_event.h>
+#include <asm/sysreg.h>
 
 #include <trace/events/kvm.h>
 
@@ -67,11 +68,9 @@ static u32 get_ccsidr(u32 csselr)
 
 	/* Make sure noone else changes CSSELR during this! */
 	local_irq_disable();
-	/* Put value into CSSELR */
-	asm volatile("msr csselr_el1, %x0" : : "r" (csselr));
+	write_sysreg(csselr, csselr_el1);
 	isb();
-	/* Read result out of CCSIDR */
-	asm volatile("mrs %0, ccsidr_el1" : "=r" (ccsidr));
+	ccsidr = read_sysreg(ccsidr_el1);
 	local_irq_enable();
 
 	return ccsidr;
@@ -174,9 +173,7 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
 	if (p->is_write) {
 		return ignore_write(vcpu, p);
 	} else {
-		u32 val;
-		asm volatile("mrs %0, dbgauthstatus_el1" : "=r" (val));
-		p->regval = val;
+		p->regval = read_sysreg(dbgauthstatus_el1);
 		return true;
 	}
 }
@@ -429,10 +426,7 @@ static void reset_wcr(struct kvm_vcpu *vcpu,
 
 static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
-	u64 amair;
-
-	asm volatile("mrs %0, amair_el1\n" : "=r" (amair));
-	vcpu_sys_reg(vcpu, AMAIR_EL1) = amair;
+	vcpu_sys_reg(vcpu, AMAIR_EL1) = read_sysreg(amair_el1);
 }
 
 static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
@@ -456,8 +450,9 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
 	u64 pmcr, val;
 
-	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
-	/* Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) is reset to UNKNOWN
+	pmcr = read_sysreg(pmcr_el0);
+	/*
+	 * Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) are reset to UNKNOWN
 	 * except PMCR.E resetting to zero.
 	 */
 	val = ((pmcr & ~ARMV8_PMU_PMCR_MASK)
@@ -557,9 +552,9 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 		return false;
 
 	if (!(p->Op2 & 1))
-		asm volatile("mrs %0, pmceid0_el0\n" : "=r" (pmceid));
+		pmceid = read_sysreg(pmceid0_el0);
 	else
-		asm volatile("mrs %0, pmceid1_el0\n" : "=r" (pmceid));
+		pmceid = read_sysreg(pmceid1_el0);
 
 	p->regval = pmceid;
 
@@ -1833,11 +1828,7 @@ static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu,
 	static void get_##reg(struct kvm_vcpu *v,			\
 			      const struct sys_reg_desc *r)		\
 	{								\
-		u64 val;						\
-									\
-		asm volatile("mrs %0, " __stringify(reg) "\n"		\
-			     : "=r" (val));				\
-		((struct sys_reg_desc *)r)->val = val;			\
+		((struct sys_reg_desc *)r)->val = read_sysreg(reg);	\
 	}
 
 FUNCTION_INVARIANT(midr_el1)
diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c b/arch/arm64/kvm/sys_regs_generic_v8.c
index ed90578..46af718 100644
--- a/arch/arm64/kvm/sys_regs_generic_v8.c
+++ b/arch/arm64/kvm/sys_regs_generic_v8.c
@@ -26,6 +26,7 @@
 #include <asm/kvm_host.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_coproc.h>
+#include <asm/sysreg.h>
 #include <linux/init.h>
 
 #include "sys_regs.h"
@@ -43,10 +44,7 @@ static bool access_actlr(struct kvm_vcpu *vcpu,
 
 static void reset_actlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
-	u64 actlr;
-
-	asm volatile("mrs %0, actlr_el1\n" : "=r" (actlr));
-	vcpu_sys_reg(vcpu, ACTLR_EL1) = actlr;
+	vcpu_sys_reg(vcpu, ACTLR_EL1) = read_sysreg(actlr_el1);
 }
 
 /*
-- 
1.9.1

[PATCH 5/6] arm64: simplify sysreg manipulation

[Mark Rutland]

A while back we added {read,write}_sysreg accessors to handle accesses
to system registers, without the usual boilerplate asm volatile,
temporary variable, etc.

This patch makes use of these across arm64 to make code shorter and
clearer. For sequences with a trailing ISB, the existing isb() macro is
also used so that asm blocks can be removed entirely.

A few uses of inline assembly for msr/mrs are left as-is. Those
manipulating sp_el0 for the current thread_info value have special
clobber requiremends.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/hw_breakpoint.h | 13 +++++++------
 arch/arm64/include/asm/mmu_context.h   | 27 ++++++++++-----------------
 arch/arm64/include/asm/pgtable-hwdef.h |  1 +
 arch/arm64/include/asm/sysreg.h        | 20 ++++++++++----------
 arch/arm64/include/asm/thread_info.h   |  3 +++
 arch/arm64/kernel/cacheinfo.c          |  8 +++-----
 arch/arm64/kernel/debug-monitors.c     |  8 +++-----
 arch/arm64/kernel/process.c            | 14 ++++++--------
 arch/arm64/kernel/sys_compat.c         |  2 +-
 9 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 115ea2a..783ba7d 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -18,6 +18,7 @@
 
 #include <asm/cputype.h>
 #include <asm/cpufeature.h>
+#include <asm/sysreg.h>
 #include <asm/virt.h>
 
 #ifdef __KERNEL__
@@ -98,18 +99,18 @@ static inline void decode_ctrl_reg(u32 reg,
 #define AARCH64_DBG_REG_WCR	(AARCH64_DBG_REG_WVR + ARM_MAX_WRP)
 
 /* Debug register names. */
-#define AARCH64_DBG_REG_NAME_BVR	"bvr"
-#define AARCH64_DBG_REG_NAME_BCR	"bcr"
-#define AARCH64_DBG_REG_NAME_WVR	"wvr"
-#define AARCH64_DBG_REG_NAME_WCR	"wcr"
+#define AARCH64_DBG_REG_NAME_BVR	bvr
+#define AARCH64_DBG_REG_NAME_BCR	bcr
+#define AARCH64_DBG_REG_NAME_WVR	wvr
+#define AARCH64_DBG_REG_NAME_WCR	wcr
 
 /* Accessor macros for the debug registers. */
 #define AARCH64_DBG_READ(N, REG, VAL) do {\
-	asm volatile("mrs %0, dbg" REG #N "_el1" : "=r" (VAL));\
+	VAL = read_sysreg(dbg##REG##N##_el1);\
 } while (0)
 
 #define AARCH64_DBG_WRITE(N, REG, VAL) do {\
-	asm volatile("msr dbg" REG #N "_el1, %0" :: "r" (VAL));\
+	write_sysreg(VAL, dbg##REG##N##_el1);\
 } while (0)
 
 struct task_struct;
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index b1892a0..e5c24b4 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -27,16 +27,14 @@
 #include <asm-generic/mm_hooks.h>
 #include <asm/cputype.h>
 #include <asm/pgtable.h>
+#include <asm/sysreg.h>
 #include <asm/tlbflush.h>
 
 #ifdef CONFIG_PID_IN_CONTEXTIDR
 static inline void contextidr_thread_switch(struct task_struct *next)
 {
-	asm(
-	"	msr	contextidr_el1, %0\n"
-	"	isb"
-	:
-	: "r" (task_pid_nr(next)));
+	write_sysreg(task_pid_nr(next), contextidr_el1);
+	isb();
 }
 #else
 static inline void contextidr_thread_switch(struct task_struct *next)
@@ -51,11 +49,8 @@ static inline void cpu_set_reserved_ttbr0(void)
 {
 	unsigned long ttbr = virt_to_phys(empty_zero_page);
 
-	asm(
-	"	msr	ttbr0_el1, %0			// set TTBR0\n"
-	"	isb"
-	:
-	: "r" (ttbr));
+	write_sysreg(ttbr, ttbr0_el1);
+	isb();
 }
 
 /*
@@ -81,13 +76,11 @@ static inline void __cpu_set_tcr_t0sz(unsigned long t0sz)
 	if (!__cpu_uses_extended_idmap())
 		return;
 
-	asm volatile (
-	"	mrs	%0, tcr_el1	;"
-	"	bfi	%0, %1, %2, %3	;"
-	"	msr	tcr_el1, %0	;"
-	"	isb"
-	: "=&r" (tcr)
-	: "r"(t0sz), "I"(TCR_T0SZ_OFFSET), "I"(TCR_TxSZ_WIDTH));
+	tcr = read_sysreg(tcr_el1);
+	tcr &= ~TCR_T0SZ_MASK;
+	tcr |= t0sz << TCR_T0SZ_OFFSET;
+	write_sysreg(tcr, tcr_el1);
+	isb();
 }
 
 #define cpu_set_default_tcr_t0sz()	__cpu_set_tcr_t0sz(TCR_T0SZ(VA_BITS))
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index c3ae239..eb0c2bd 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -208,6 +208,7 @@
 #define TCR_T1SZ(x)		((UL(64) - (x)) << TCR_T1SZ_OFFSET)
 #define TCR_TxSZ(x)		(TCR_T0SZ(x) | TCR_T1SZ(x))
 #define TCR_TxSZ_WIDTH		6
+#define TCR_T0SZ_MASK		(((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T0SZ_OFFSET)
 
 #define TCR_IRGN0_SHIFT		8
 #define TCR_IRGN0_MASK		(UL(3) << TCR_IRGN0_SHIFT)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 39fed2e..e91aef2b 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -253,16 +253,6 @@ asm(
 "	.endm\n"
 );
 
-static inline void config_sctlr_el1(u32 clear, u32 set)
-{
-	u32 val;
-
-	asm volatile("mrs %0, sctlr_el1" : "=r" (val));
-	val &= ~clear;
-	val |= set;
-	asm volatile("msr sctlr_el1, %0" : : "r" (val));
-}
-
 /*
  * Unlike read_cpuid, calls to read_sysreg are never expected to be
  * optimized away or replaced with synthetic values.
@@ -283,6 +273,16 @@ static inline void config_sctlr_el1(u32 clear, u32 set)
 		     : : "rZ" (__val));				\
 } while (0)
 
+static inline void config_sctlr_el1(u32 clear, u32 set)
+{
+	u32 val;
+
+	val = read_sysreg(sctlr_el1);
+	val &= ~clear;
+	val |= set;
+	write_sysreg(val, sctlr_el1);
+}
+
 #endif
 
 #endif	/* __ASM_SYSREG_H */
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index abd64bd..e9ea5a6 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -75,6 +75,9 @@ static inline struct thread_info *current_thread_info(void) __attribute_const__;
 
 /*
  * struct thread_info can be accessed directly via sp_el0.
+ *
+ * We don't use read_sysreg() as we want the compiler to cache the value where
+ * possible.
  */
 static inline struct thread_info *current_thread_info(void)
 {
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index b8629d5..9617301 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -39,7 +39,7 @@ static inline enum cache_type get_cache_type(int level)
 
 	if (level > MAX_CACHE_LEVEL)
 		return CACHE_TYPE_NOCACHE;
-	asm volatile ("mrs     %x0, clidr_el1" : "=r" (clidr));
+	clidr = read_sysreg(clidr_el1);
 	return CLIDR_CTYPE(clidr, level);
 }
 
@@ -55,11 +55,9 @@ u64 __attribute_const__ cache_get_ccsidr(u64 csselr)
 
 	WARN_ON(preemptible());
 
-	/* Put value into CSSELR */
-	asm volatile("msr csselr_el1, %x0" : : "r" (csselr));
+	write_sysreg(csselr, csselr_el1);
 	isb();
-	/* Read result out of CCSIDR */
-	asm volatile("mrs %x0, ccsidr_el1" : "=r" (ccsidr));
+	ccsidr = read_sysreg(ccsidr_el1);
 
 	return ccsidr;
 }
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 91fff48..c2cd1c3 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -46,16 +46,14 @@ static void mdscr_write(u32 mdscr)
 {
 	unsigned long flags;
 	local_dbg_save(flags);
-	asm volatile("msr mdscr_el1, %0" :: "r" (mdscr));
+	write_sysreg(mdscr, mdscr_el1);
 	local_dbg_restore(flags);
 }
 NOKPROBE_SYMBOL(mdscr_write);
 
 static u32 mdscr_read(void)
 {
-	u32 mdscr;
-	asm volatile("mrs %0, mdscr_el1" : "=r" (mdscr));
-	return mdscr;
+	return read_sysreg(mdscr_el1);
 }
 NOKPROBE_SYMBOL(mdscr_read);
 
@@ -134,7 +132,7 @@ NOKPROBE_SYMBOL(disable_debug_monitors);
  */
 static void clear_os_lock(void *unused)
 {
-	asm volatile("msr oslar_el1, %0" : : "r" (0));
+	write_sysreg(0, oslar_el1);
 }
 
 static int os_lock_notify(struct notifier_block *self,
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6cd2612..a4f5f76 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -202,7 +202,7 @@ void show_regs(struct pt_regs * regs)
 
 static void tls_thread_flush(void)
 {
-	asm ("msr tpidr_el0, xzr");
+	write_sysreg(0, tpidr_el0);
 
 	if (is_compat_task()) {
 		current->thread.tp_value = 0;
@@ -213,7 +213,7 @@ static void tls_thread_flush(void)
 		 * with a stale shadow state during context switch.
 		 */
 		barrier();
-		asm ("msr tpidrro_el0, xzr");
+		write_sysreg(0, tpidrro_el0);
 	}
 }
 
@@ -253,7 +253,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
 		 * Read the current TLS pointer from tpidr_el0 as it may be
 		 * out-of-sync with the saved value.
 		 */
-		asm("mrs %0, tpidr_el0" : "=r" (*task_user_tls(p)));
+		*task_user_tls(p) = read_sysreg(tpidr_el0);
 
 		if (stack_start) {
 			if (is_compat_thread(task_thread_info(p)))
@@ -289,17 +289,15 @@ static void tls_thread_switch(struct task_struct *next)
 {
 	unsigned long tpidr, tpidrro;
 
-	asm("mrs %0, tpidr_el0" : "=r" (tpidr));
+	tpidr = read_sysreg(tpidr_el0);
 	*task_user_tls(current) = tpidr;
 
 	tpidr = *task_user_tls(next);
 	tpidrro = is_compat_thread(task_thread_info(next)) ?
 		  next->thread.tp_value : 0;
 
-	asm(
-	"	msr	tpidr_el0, %0\n"
-	"	msr	tpidrro_el0, %1"
-	: : "r" (tpidr), "r" (tpidrro));
+	write_sysreg(tpidr, tpidr_el0);
+	write_sysreg(tpidrro, tpidrro_el0);
 }
 
 /* Restore the UAO state depending on next's addr_limit */
diff --git a/arch/arm64/kernel/sys_compat.c b/arch/arm64/kernel/sys_compat.c
index 28c511b..abaf582 100644
--- a/arch/arm64/kernel/sys_compat.c
+++ b/arch/arm64/kernel/sys_compat.c
@@ -94,7 +94,7 @@ long compat_arm_syscall(struct pt_regs *regs)
 		 * See comment in tls_thread_flush.
 		 */
 		barrier();
-		asm ("msr tpidrro_el0, %0" : : "r" (regs->regs[0]));
+		write_sysreg(regs->regs[0], tpidrro_el0);
 		return 0;
 
 	default:
-- 
1.9.1

[PATCH 6/6] arm64: simplify contextidr_thread_switch

[Mark Rutland]

When CONFIG_PID_IN_CONTEXTIDR is not selected, we use an empty stub
definition of contextidr_thread_switch(). As everything we rely upon
exists regardless of CONFIG_PID_IN_CONTEXTIDR, we don't strictly require
an empty stub.

By using IS_ENABLED() rather than ifdeffery, we avoid duplication, and
get compiler coverage on all the code even when CONFIG_PID_IN_CONTEXTIDR
is not selected and the code is optimised away.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/mmu_context.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index e5c24b4..a501853 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -30,17 +30,14 @@
 #include <asm/sysreg.h>
 #include <asm/tlbflush.h>
 
-#ifdef CONFIG_PID_IN_CONTEXTIDR
 static inline void contextidr_thread_switch(struct task_struct *next)
 {
+	if (!IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR))
+		return;
+
 	write_sysreg(task_pid_nr(next), contextidr_el1);
 	isb();
 }
-#else
-static inline void contextidr_thread_switch(struct task_struct *next)
-{
-}
-#endif
 
 /*
  * Set TTBR0 to empty_zero_page. No translations will be possible via TTBR0.
-- 
1.9.1

[PATCH 4/6] arm64/kvm: use {read,write}_sysreg()

[Christoffer Dall]

On Thu, Sep 08, 2016 at 01:55:37PM +0100, Mark Rutland wrote:
> A while back we added {read,write}_sysreg accessors to handle accesses
> to system registers, without the usual boilerplate asm volatile,
> temporary variable, etc.
> 
> This patch makes use of these in the arm64 KVM code to make the code
> shorter and clearer.
> 
> At the same time, a comment style violation next to a system register
> access is fixed up in reset_pmcr, and comments describing whether
> operations are reads or writes are removed as this is now painfully
> obvious.

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/virt.h        |  6 ++----
>  arch/arm64/kvm/sys_regs.c            | 31 +++++++++++--------------------
>  arch/arm64/kvm/sys_regs_generic_v8.c |  6 ++----
>  3 files changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> index 1788545..c5cc940 100644
> --- a/arch/arm64/include/asm/virt.h
> +++ b/arch/arm64/include/asm/virt.h
> @@ -45,6 +45,7 @@
>  #ifndef __ASSEMBLY__
>  
>  #include <asm/ptrace.h>
> +#include <asm/sysreg.h>
>  
>  /*
>   * __boot_cpu_mode records what mode CPUs were booted in.
> @@ -75,10 +76,7 @@ static inline bool is_hyp_mode_mismatched(void)
>  
>  static inline bool is_kernel_in_hyp_mode(void)
>  {
> -	u64 el;
> -
> -	asm("mrs %0, CurrentEL" : "=r" (el));
> -	return el == CurrentEL_EL2;
> +	return read_sysreg(CurrentEL) == CurrentEL_EL2;
>  }
>  
>  #ifdef CONFIG_ARM64_VHE
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e51367d..f302fdb 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -36,6 +36,7 @@
>  #include <asm/kvm_host.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/perf_event.h>
> +#include <asm/sysreg.h>
>  
>  #include <trace/events/kvm.h>
>  
> @@ -67,11 +68,9 @@ static u32 get_ccsidr(u32 csselr)
>  
>  	/* Make sure noone else changes CSSELR during this! */
>  	local_irq_disable();
> -	/* Put value into CSSELR */
> -	asm volatile("msr csselr_el1, %x0" : : "r" (csselr));
> +	write_sysreg(csselr, csselr_el1);
>  	isb();
> -	/* Read result out of CCSIDR */
> -	asm volatile("mrs %0, ccsidr_el1" : "=r" (ccsidr));
> +	ccsidr = read_sysreg(ccsidr_el1);
>  	local_irq_enable();
>  
>  	return ccsidr;
> @@ -174,9 +173,7 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
>  	if (p->is_write) {
>  		return ignore_write(vcpu, p);
>  	} else {
> -		u32 val;
> -		asm volatile("mrs %0, dbgauthstatus_el1" : "=r" (val));
> -		p->regval = val;
> +		p->regval = read_sysreg(dbgauthstatus_el1);
>  		return true;
>  	}
>  }
> @@ -429,10 +426,7 @@ static void reset_wcr(struct kvm_vcpu *vcpu,
>  
>  static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  {
> -	u64 amair;
> -
> -	asm volatile("mrs %0, amair_el1\n" : "=r" (amair));
> -	vcpu_sys_reg(vcpu, AMAIR_EL1) = amair;
> +	vcpu_sys_reg(vcpu, AMAIR_EL1) = read_sysreg(amair_el1);
>  }
>  
>  static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> @@ -456,8 +450,9 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  {
>  	u64 pmcr, val;
>  
> -	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> -	/* Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) is reset to UNKNOWN
> +	pmcr = read_sysreg(pmcr_el0);
> +	/*
> +	 * Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) are reset to UNKNOWN
>  	 * except PMCR.E resetting to zero.
>  	 */
>  	val = ((pmcr & ~ARMV8_PMU_PMCR_MASK)
> @@ -557,9 +552,9 @@ static bool access_pmceid(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  		return false;
>  
>  	if (!(p->Op2 & 1))
> -		asm volatile("mrs %0, pmceid0_el0\n" : "=r" (pmceid));
> +		pmceid = read_sysreg(pmceid0_el0);
>  	else
> -		asm volatile("mrs %0, pmceid1_el0\n" : "=r" (pmceid));
> +		pmceid = read_sysreg(pmceid1_el0);
>  
>  	p->regval = pmceid;
>  
> @@ -1833,11 +1828,7 @@ static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu,
>  	static void get_##reg(struct kvm_vcpu *v,			\
>  			      const struct sys_reg_desc *r)		\
>  	{								\
> -		u64 val;						\
> -									\
> -		asm volatile("mrs %0, " __stringify(reg) "\n"		\
> -			     : "=r" (val));				\
> -		((struct sys_reg_desc *)r)->val = val;			\
> +		((struct sys_reg_desc *)r)->val = read_sysreg(reg);	\
>  	}
>  
>  FUNCTION_INVARIANT(midr_el1)
> diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c b/arch/arm64/kvm/sys_regs_generic_v8.c
> index ed90578..46af718 100644
> --- a/arch/arm64/kvm/sys_regs_generic_v8.c
> +++ b/arch/arm64/kvm/sys_regs_generic_v8.c
> @@ -26,6 +26,7 @@
>  #include <asm/kvm_host.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_coproc.h>
> +#include <asm/sysreg.h>
>  #include <linux/init.h>
>  
>  #include "sys_regs.h"
> @@ -43,10 +44,7 @@ static bool access_actlr(struct kvm_vcpu *vcpu,
>  
>  static void reset_actlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  {
> -	u64 actlr;
> -
> -	asm volatile("mrs %0, actlr_el1\n" : "=r" (actlr));
> -	vcpu_sys_reg(vcpu, ACTLR_EL1) = actlr;
> +	vcpu_sys_reg(vcpu, ACTLR_EL1) = read_sysreg(actlr_el1);
>  }
>  
>  /*
> -- 
> 1.9.1
> 

[PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR

[Robin Murphy]

On 08/09/16 13:55, Mark Rutland wrote:
> Currently write_sysreg has to allocate a temporary register to write
> zero to a system register, which is unfortunate given that the MSR
> instruction accepts XZR as an operand.
> 
> Allow XZR to be used when appropriate by fiddling with the assembly
> constraints.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/sysreg.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index cc06794..39fed2e 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -273,10 +273,14 @@ static inline void config_sctlr_el1(u32 clear, u32 set)
>  	__val;							\
>  })
>  
> +/*
> + * The "Z" constraint normally means a zero immediate, but when combined with
> + * the "%x0" template means XZR.
> + */

Personally I find that comment more confusing than useful - "Z" is
documented for AArch64 as "Integer constant zero", which to me doesn't
necessarily imply an immediate; it's just not necessarily immediately
intuitive that that can be an immediate constant or a 'register
constant' depending on exactly how it is referenced.

Either way, though:

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

>  #define write_sysreg(v, r) do {					\
>  	u64 __val = (u64)v;					\
> -	asm volatile("msr " __stringify(r) ", %0"		\
> -		     : : "r" (__val));				\
> +	asm volatile("msr " __stringify(r) ", %x0"		\
> +		     : : "rZ" (__val));				\
>  } while (0)
>  
>  #endif
>