* [PATCH 1/3] arm64: add macro to extract ESR_ELx.EC
2016-05-31 11:33 [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Mark Rutland
@ 2016-05-31 11:33 ` Mark Rutland
2016-05-31 11:33 ` [PATCH 2/3] arm64/kvm: use ESR_ELx_EC to extract EC Mark Rutland
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-05-31 11:33 UTC (permalink / raw)
To: linux-arm-kernel
Several places open-code extraction of the EC field from an ESR_ELx
value, in subtly different ways. This is unfortunate duplication and
variation, and the precise logic used to extract the field is a
distraction.
This patch adds a new macro, ESR_ELx_EC(), to extract the EC field from
an ESR_ELx value in a consistent fashion.
Existing open-coded extractions in core arm64 code are moved over to the
new helper. KVM code is left as-is for the moment.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Huang Shijie <shijie.huang@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave P Martin <dave.martin@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/include/asm/esr.h | 1 +
arch/arm64/kernel/traps.c | 2 +-
arch/arm64/mm/fault.c | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 77eeb2c..f772e15 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -74,6 +74,7 @@
#define ESR_ELx_EC_SHIFT (26)
#define ESR_ELx_EC_MASK (UL(0x3F) << ESR_ELx_EC_SHIFT)
+#define ESR_ELx_EC(esr) (((esr) & ESR_ELx_EC_MASK) >> ESR_ELx_EC_SHIFT)
#define ESR_ELx_IL (UL(1) << 25)
#define ESR_ELx_ISS_MASK (ESR_ELx_IL - 1)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c539208..673d8d7 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -465,7 +465,7 @@ static const char *esr_class_str[] = {
const char *esr_get_class_string(u32 esr)
{
- return esr_class_str[esr >> ESR_ELx_EC_SHIFT];
+ return esr_class_str[ESR_ELx_EC(esr)];
}
/*
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 5954881..6b6f5ec 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -244,7 +244,7 @@ out:
static inline int permission_fault(unsigned int esr)
{
- unsigned int ec = (esr & ESR_ELx_EC_MASK) >> ESR_ELx_EC_SHIFT;
+ unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
return (ec == ESR_ELx_EC_DABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] arm64/kvm: use ESR_ELx_EC to extract EC
2016-05-31 11:33 [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Mark Rutland
2016-05-31 11:33 ` [PATCH 1/3] arm64: add macro to extract ESR_ELx.EC Mark Rutland
@ 2016-05-31 11:33 ` Mark Rutland
2016-05-31 14:35 ` Christoffer Dall
2016-05-31 11:33 ` [PATCH 3/3] arm64: kill ESR_LNX_EXEC Mark Rutland
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Mark Rutland @ 2016-05-31 11:33 UTC (permalink / raw)
To: linux-arm-kernel
Now that we have a helper to extract the EC from an ESR_ELx value, make
use of this in the arm64 KVM code for simplicity and consistency. There
should be no functional changes as a result of this patch.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Dave P Martin <dave.martin@arm.com>
Cc: Huang Shijie <shijie.huang@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: kvmarm at lists.cs.columbia.edu
---
arch/arm64/include/asm/kvm_emulate.h | 2 +-
arch/arm64/kvm/handle_exit.c | 4 ++--
arch/arm64/kvm/hyp/switch.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 40bc168..4cdeae3 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -210,7 +210,7 @@ static inline bool kvm_vcpu_trap_il_is32bit(const struct kvm_vcpu *vcpu)
static inline u8 kvm_vcpu_trap_get_class(const struct kvm_vcpu *vcpu)
{
- return kvm_vcpu_get_hsr(vcpu) >> ESR_ELx_EC_SHIFT;
+ return ESR_ELx_EC(kvm_vcpu_get_hsr(vcpu));
}
static inline bool kvm_vcpu_trap_is_iabt(const struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 3246c4a..fa96fe2 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -106,7 +106,7 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
run->exit_reason = KVM_EXIT_DEBUG;
run->debug.arch.hsr = hsr;
- switch (hsr >> ESR_ELx_EC_SHIFT) {
+ switch (ESR_ELx_EC(hsr)) {
case ESR_ELx_EC_WATCHPT_LOW:
run->debug.arch.far = vcpu->arch.fault.far_el2;
/* fall through */
@@ -149,7 +149,7 @@ static exit_handle_fn arm_exit_handlers[] = {
static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
{
u32 hsr = kvm_vcpu_get_hsr(vcpu);
- u8 hsr_ec = hsr >> ESR_ELx_EC_SHIFT;
+ u8 hsr_ec = ESR_ELx_EC(hsr);
if (hsr_ec >= ARRAY_SIZE(arm_exit_handlers) ||
!arm_exit_handlers[hsr_ec]) {
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 437cfad..4373997 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -198,7 +198,7 @@ static bool __hyp_text __translate_far_to_hpfar(u64 far, u64 *hpfar)
static bool __hyp_text __populate_fault_info(struct kvm_vcpu *vcpu)
{
u64 esr = read_sysreg_el2(esr);
- u8 ec = esr >> ESR_ELx_EC_SHIFT;
+ u8 ec = ESR_ELx_EC(esr);
u64 hpfar, far;
vcpu->arch.fault.esr_el2 = esr;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] arm64/kvm: use ESR_ELx_EC to extract EC
2016-05-31 11:33 ` [PATCH 2/3] arm64/kvm: use ESR_ELx_EC to extract EC Mark Rutland
@ 2016-05-31 14:35 ` Christoffer Dall
0 siblings, 0 replies; 9+ messages in thread
From: Christoffer Dall @ 2016-05-31 14:35 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 31, 2016 at 12:33:02PM +0100, Mark Rutland wrote:
> Now that we have a helper to extract the EC from an ESR_ELx value, make
> use of this in the arm64 KVM code for simplicity and consistency. There
> should be no functional changes as a result of this patch.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Dave P Martin <dave.martin@arm.com>
> Cc: Huang Shijie <shijie.huang@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: kvmarm at lists.cs.columbia.edu
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 2 +-
> arch/arm64/kvm/handle_exit.c | 4 ++--
> arch/arm64/kvm/hyp/switch.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 40bc168..4cdeae3 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -210,7 +210,7 @@ static inline bool kvm_vcpu_trap_il_is32bit(const struct kvm_vcpu *vcpu)
>
> static inline u8 kvm_vcpu_trap_get_class(const struct kvm_vcpu *vcpu)
> {
> - return kvm_vcpu_get_hsr(vcpu) >> ESR_ELx_EC_SHIFT;
> + return ESR_ELx_EC(kvm_vcpu_get_hsr(vcpu));
> }
>
> static inline bool kvm_vcpu_trap_is_iabt(const struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 3246c4a..fa96fe2 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -106,7 +106,7 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->exit_reason = KVM_EXIT_DEBUG;
> run->debug.arch.hsr = hsr;
>
> - switch (hsr >> ESR_ELx_EC_SHIFT) {
> + switch (ESR_ELx_EC(hsr)) {
> case ESR_ELx_EC_WATCHPT_LOW:
> run->debug.arch.far = vcpu->arch.fault.far_el2;
> /* fall through */
> @@ -149,7 +149,7 @@ static exit_handle_fn arm_exit_handlers[] = {
> static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
> {
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
> - u8 hsr_ec = hsr >> ESR_ELx_EC_SHIFT;
> + u8 hsr_ec = ESR_ELx_EC(hsr);
>
> if (hsr_ec >= ARRAY_SIZE(arm_exit_handlers) ||
> !arm_exit_handlers[hsr_ec]) {
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 437cfad..4373997 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -198,7 +198,7 @@ static bool __hyp_text __translate_far_to_hpfar(u64 far, u64 *hpfar)
> static bool __hyp_text __populate_fault_info(struct kvm_vcpu *vcpu)
> {
> u64 esr = read_sysreg_el2(esr);
> - u8 ec = esr >> ESR_ELx_EC_SHIFT;
> + u8 ec = ESR_ELx_EC(esr);
> u64 hpfar, far;
>
> vcpu->arch.fault.esr_el2 = esr;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] arm64: kill ESR_LNX_EXEC
2016-05-31 11:33 [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Mark Rutland
2016-05-31 11:33 ` [PATCH 1/3] arm64: add macro to extract ESR_ELx.EC Mark Rutland
2016-05-31 11:33 ` [PATCH 2/3] arm64/kvm: use ESR_ELx_EC to extract EC Mark Rutland
@ 2016-05-31 11:33 ` Mark Rutland
2016-05-31 16:53 ` [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Catalin Marinas
2016-06-21 16:09 ` Catalin Marinas
4 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-05-31 11:33 UTC (permalink / raw)
To: linux-arm-kernel
Currently we treat ESR_EL1 bit 24 as software-defined for distinguishing
instruction aborts from data aborts, but this bit is architecturally
RES0 for instruction aborts, and could be allocated for an arbitrary
purpose in future. Additionally, we hard-code the value in entry.S
without the mnemonic, making the code difficult to understand.
Instead, remove ESR_LNX_EXEC, and distinguish aborts based on the esr,
which we already pass to the sole use of ESR_LNX_EXEC. A new helper,
is_el0_instruction_abort() is added to make the logic clear. Any
instruction aborts taken from EL1 will already have been handled by
bad_mode, so we need not handle that case in the helper.
For consistency, the existing permission_fault helper is renamed to
is_permission_fault, and the return type is changed to bool. There
should be no functional changes as the return value was a boolean
expression, and the result is only used in another boolean expression.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave P Martin <dave.martin@arm.com>
Cc: Huang Shijie <shijie.huang@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/entry.S | 2 +-
arch/arm64/mm/fault.c | 13 ++++++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 12e8d2b..eefffa8 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -532,7 +532,7 @@ el0_ia:
enable_dbg_and_irq
ct_user_exit
mov x0, x26
- orr x1, x25, #1 << 24 // use reserved ISS bit for instruction aborts
+ mov x1, x25
mov x2, sp
bl do_mem_abort
b ret_to_user
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 6b6f5ec..5af5f7f 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -202,8 +202,6 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
#define VM_FAULT_BADMAP 0x010000
#define VM_FAULT_BADACCESS 0x020000
-#define ESR_LNX_EXEC (1 << 24)
-
static int __do_page_fault(struct mm_struct *mm, unsigned long addr,
unsigned int mm_flags, unsigned long vm_flags,
struct task_struct *tsk)
@@ -242,7 +240,7 @@ out:
return fault;
}
-static inline int permission_fault(unsigned int esr)
+static inline bool is_permission_fault(unsigned int esr)
{
unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
@@ -250,6 +248,11 @@ static inline int permission_fault(unsigned int esr)
return (ec == ESR_ELx_EC_DABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
}
+static bool is_el0_instruction_abort(unsigned int esr)
+{
+ return ESR_ELx_EC(esr) == ESR_ELx_EC_IABT_LOW;
+}
+
static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
{
@@ -272,14 +275,14 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
if (user_mode(regs))
mm_flags |= FAULT_FLAG_USER;
- if (esr & ESR_LNX_EXEC) {
+ if (is_el0_instruction_abort(esr)) {
vm_flags = VM_EXEC;
} else if ((esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM)) {
vm_flags = VM_WRITE;
mm_flags |= FAULT_FLAG_WRITE;
}
- if (permission_fault(esr) && (addr < USER_DS)) {
+ if (is_permission_fault(esr) && (addr < USER_DS)) {
if (get_fs() == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 0/3] arm64: Kill ESR_LNX_EXEC
2016-05-31 11:33 [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Mark Rutland
` (2 preceding siblings ...)
2016-05-31 11:33 ` [PATCH 3/3] arm64: kill ESR_LNX_EXEC Mark Rutland
@ 2016-05-31 16:53 ` Catalin Marinas
2016-06-02 18:21 ` Will Deacon
2016-06-21 16:09 ` Catalin Marinas
4 siblings, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2016-05-31 16:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 31, 2016 at 12:33:00PM +0100, Mark Rutland wrote:
> Mark Rutland (3):
> arm64: add macro to extract ESR_ELx.EC
> arm64/kvm: use ESR_ELx_EC to extract EC
> arm64: kill ESR_LNX_EXEC
For the series:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 0/3] arm64: Kill ESR_LNX_EXEC
2016-05-31 16:53 ` [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Catalin Marinas
@ 2016-06-02 18:21 ` Will Deacon
2016-06-06 10:59 ` Mark Rutland
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2016-06-02 18:21 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 31, 2016 at 05:53:11PM +0100, Catalin Marinas wrote:
> On Tue, May 31, 2016 at 12:33:00PM +0100, Mark Rutland wrote:
> > Mark Rutland (3):
> > arm64: add macro to extract ESR_ELx.EC
> > arm64/kvm: use ESR_ELx_EC to extract EC
> > arm64: kill ESR_LNX_EXEC
>
> For the series:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
You may as well queue this for 4.8 when you start picking up patches.
I don't think it warrants going in for 4.7, since it doesn't actually
fix anything.
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/3] arm64: Kill ESR_LNX_EXEC
2016-06-02 18:21 ` Will Deacon
@ 2016-06-06 10:59 ` Mark Rutland
0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2016-06-06 10:59 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 02, 2016 at 07:21:47PM +0100, Will Deacon wrote:
> On Tue, May 31, 2016 at 05:53:11PM +0100, Catalin Marinas wrote:
> > On Tue, May 31, 2016 at 12:33:00PM +0100, Mark Rutland wrote:
> > > Mark Rutland (3):
> > > arm64: add macro to extract ESR_ELx.EC
> > > arm64/kvm: use ESR_ELx_EC to extract EC
> > > arm64: kill ESR_LNX_EXEC
> >
> > For the series:
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> You may as well queue this for 4.8 when you start picking up patches.
> I don't think it warrants going in for 4.7, since it doesn't actually
> fix anything.
Makes sense to me.
Catalin, I've dropped this in my git repo for the moment [1], and can
rebase/repost as/when required. Please give me a poke if so.
Thanks,
Mark.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/kill-esr-lnx-exec
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/3] arm64: Kill ESR_LNX_EXEC
2016-05-31 11:33 [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Mark Rutland
` (3 preceding siblings ...)
2016-05-31 16:53 ` [PATCH 0/3] arm64: Kill ESR_LNX_EXEC Catalin Marinas
@ 2016-06-21 16:09 ` Catalin Marinas
4 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2016-06-21 16:09 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 31, 2016 at 12:33:00PM +0100, Mark Rutland wrote:
> Currently we (ab)use a reserved bit in ESR_ELx for our own purposes as
> ESR_LNX_EXEC, which isn't ideal, especially as we're inconsistent with our
> mnemonic usage. This series removes ESR_LNX_EXEC entirely, avoiding (ab)use of
> reserved ESR_ELx bits, and makes things a little more consistent.
>
> I've extracted this from my entry-deasm branch [1], and rebased to v4.7-rc1.
> I've split the KVM changes as the KVM code is undergoing rapid change these
> days, and it should be possible to take those as a subsequent cleanup rather
> than forcing a painful merging process.
>
> Thanks,
> Mark.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=arm64/entry-deasm
>
>
> Mark Rutland (3):
> arm64: add macro to extract ESR_ELx.EC
> arm64/kvm: use ESR_ELx_EC to extract EC
> arm64: kill ESR_LNX_EXEC
I queued these 3 patches for 4.8. Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread