* [PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
2016-09-08 14:12 ` Robin Murphy
2016-09-08 12:55 ` [PATCH 2/6] arm64: arch_timer: simplify accessors Mark Rutland
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR
2016-09-08 12:55 ` [PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR Mark Rutland
@ 2016-09-08 14:12 ` Robin Murphy
0 siblings, 0 replies; 9+ messages in thread
From: Robin Murphy @ 2016-09-08 14:12 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/6] arm64: arch_timer: simplify accessors
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
2016-09-08 12:55 ` [PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
2016-09-08 12:55 ` [PATCH 3/6] arm64: dcc: " Mark Rutland
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/6] arm64: dcc: simplify accessors
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
2016-09-08 12:55 ` [PATCH 1/6] arm64: sysreg: allow write_sysreg to use XZR Mark Rutland
2016-09-08 12:55 ` [PATCH 2/6] arm64: arch_timer: simplify accessors Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
2016-09-08 12:55 ` [PATCH 4/6] arm64/kvm: use {read,write}_sysreg() Mark Rutland
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] arm64/kvm: use {read,write}_sysreg()
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
` (2 preceding siblings ...)
2016-09-08 12:55 ` [PATCH 3/6] arm64: dcc: " Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
2016-09-08 13:34 ` Christoffer Dall
2016-09-08 12:55 ` [PATCH 5/6] arm64: simplify sysreg manipulation Mark Rutland
2016-09-08 12:55 ` [PATCH 6/6] arm64: simplify contextidr_thread_switch Mark Rutland
5 siblings, 1 reply; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] arm64/kvm: use {read,write}_sysreg()
2016-09-08 12:55 ` [PATCH 4/6] arm64/kvm: use {read,write}_sysreg() Mark Rutland
@ 2016-09-08 13:34 ` Christoffer Dall
0 siblings, 0 replies; 9+ messages in thread
From: Christoffer Dall @ 2016-09-08 13:34 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/6] arm64: simplify sysreg manipulation
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
` (3 preceding siblings ...)
2016-09-08 12:55 ` [PATCH 4/6] arm64/kvm: use {read,write}_sysreg() Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
2016-09-08 12:55 ` [PATCH 6/6] arm64: simplify contextidr_thread_switch Mark Rutland
5 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/6] arm64: simplify contextidr_thread_switch
2016-09-08 12:55 [PATCH 0/6] arm64: replace open-coded {msr, mrs} with {read, write}_sysreg() Mark Rutland
` (4 preceding siblings ...)
2016-09-08 12:55 ` [PATCH 5/6] arm64: simplify sysreg manipulation Mark Rutland
@ 2016-09-08 12:55 ` Mark Rutland
5 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-09-08 12:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread