* [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort
@ 2017-10-17 14:23 Dongjiu Geng
2017-10-18 10:09 ` Christoffer Dall
2017-10-18 10:19 ` Marc Zyngier
0 siblings, 2 replies; 4+ messages in thread
From: Dongjiu Geng @ 2017-10-17 14:23 UTC (permalink / raw)
To: will.deacon, christoffer.dall, marc.zyngier, rkrcmar,
catalin.marinas, linux-arm-kernel, kvmarm, kvm, linux-kernel,
zhanghaibin7, gengdongjiu
When a exception is trapped to EL2, hardware uses ELR_ELx to hold
the current fault instruction address. If KVM wants to inject a
abort to 32 bit guest, it needs to set the LR register for the
guest to emulate this abort happened in the guest. Because ARM32
architecture is pipelined execution, so the LR value has an offset to
the fault instruction address.
The offsets applied to Link value for exceptions as shown below,
which should be added for the ARM32 link register(LR).
Table taken from ARMv8 ARM DDI0487B-B, table G1-10:
Exception Offset, for PE state of:
A32 T32
Undefined Instruction +4 +2
Prefetch Abort +4 +4
Data Abort +8 +8
IRQ or FIQ +4 +4
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Tested-by: Haibin Zhang <zhanghaibin7@huawei.com>
---
Have tested in both arm32 and arm64
For example, in the arm64 platform, to the undefined instruction injection:
1. Guest OS call SMC(Secure Monitor Call) instruction in the address
0xc025405c, then Guest traps to hypervisor
c0254050: e59d5028 ldr r5, [sp, #40] ; 0x28
c0254054: e3a03001 mov r3, #1
c0254058: e1a01003 mov r1, r3
c025405c: e1600070 smc 0
c0254060: e30a0270 movw r0, #41584 ; 0xa270
c0254064: e34c00bf movt r0, #49343 ; 0xc0bf
2. KVM injects undefined abort to guest
3. We will find the fault PC is 0xc0254058, not 0xc025405c.
[ 12.348072] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[ 12.349786] Modules linked in:
[ 12.350563] CPU: 1 PID: 71 Comm: cat Not tainted 4.1.0-dirty #25
[ 12.352061] Hardware name: Generic DT based system
[ 12.353275] task: d9d08000 ti: d9cfe000 task.ti: d9cfe000
[ 12.354637] PC is at proc_dointvec+0x20/0x60
[ 12.355717] LR is at proc_sys_call_handler+0xb0/0xc4
[ 12.356972] pc : [<c0254058>] lr : [<c035699c>] psr: a0060013
[ 12.356972] sp : d9cffe90 ip : c0254038 fp : 00000001
[ 12.359824] r10: d9cfff80 r9 : 00000004 r8 : 00000000
[ 12.361132] r7 : bec21cb0 r6 : d9cffec4 r5 : d9cfff80 r4 : c0e82de0
[ 12.362766] r3 : 00000001 r2 : bec21cb0 r1 : 00000001 r0 : c0e82de0
[ 12.364400] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 12.366183] Control: 10c5383d Table: 59d3406a DAC: 00000015
[ 12.367623] Process cat (pid: 71, stack limit = 0xd9cfe220)
4. After correct the LR register, it will have right value
[ 125.763370] Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
[ 125.767010] Modules linked in:
[ 125.768472] CPU: 1 PID: 74 Comm: cat Tainted: G D 4.1.0-dirty #25
[ 125.771854] Hardware name: Generic DT based system
[ 125.774053] task: db0bb900 ti: d9d10000 task.ti: d9d10000
[ 125.776821] PC is at proc_dointvec+0x24/0x60
[ 125.778919] LR is at proc_sys_call_handler+0xb0/0xc4
[ 125.781269] pc : [<c025405c>] lr : [<c035699c>] psr: a0060013
[ 125.781269] sp : d9d11e90 ip : c0254038 fp : 00000001
[ 125.786581] r10: d9d11f80 r9 : 00000004 r8 : 00000000
[ 125.789673] r7 : be92ccb0 r6 : d9d11ec4 r5 : d9d11f80 r4 : c0e82de0
[ 125.792828] r3 : 00000001 r2 : be92ccb0 r1 : 00000001 r0 : c0e82de0
[ 125.795890] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
For other exception injection, such as Data/Prefetch abort, also needs to correct
---
arch/arm/kvm/emulate.c | 4 ++--
arch/arm64/kvm/inject_fault.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
index 0064b86..2419328 100644
--- a/arch/arm/kvm/emulate.c
+++ b/arch/arm/kvm/emulate.c
@@ -227,7 +227,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
u32 return_offset = (is_thumb) ? 2 : 4;
kvm_update_psr(vcpu, UND_MODE);
- *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
+ *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
/* Branch to exception vector */
*vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
@@ -242,7 +242,7 @@ static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
unsigned long cpsr = *vcpu_cpsr(vcpu);
bool is_thumb = (cpsr & PSR_T_BIT);
u32 vect_offset;
- u32 return_offset = (is_thumb) ? 4 : 0;
+ u32 return_offset = (is_pabt) ? 4 : 8;
bool is_lpae;
kvm_update_psr(vcpu, ABT_MODE);
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index da6a8cf..0416f18 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -33,12 +33,26 @@
#define LOWER_EL_AArch64_VECTOR 0x400
#define LOWER_EL_AArch32_VECTOR 0x600
+/*
+ * Table taken from ARMv8 ARM DDI0487B-B, table G1-10.
+ */
+static u8 return_offsets[8][2] = {
+ [0] = { 0, 0 }, /* Reset, unused */
+ [1] = { 4, 2 }, /* Undefined */
+ [2] = { 0, 0 }, /* SVC, unused */
+ [3] = { 4, 4 }, /* Prefetch abort */
+ [4] = { 8, 8 }, /* Data abort */
+ [5] = { 0, 0 }, /* HVC, unused */
+ [6] = { 4, 4 }, /* IRQ, unused */
+ [7] = { 4, 4 }, /* FIQ, unused */
+};
+
static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
{
unsigned long cpsr;
unsigned long new_spsr_value = *vcpu_cpsr(vcpu);
bool is_thumb = (new_spsr_value & COMPAT_PSR_T_BIT);
- u32 return_offset = (is_thumb) ? 4 : 0;
+ u32 return_offset = return_offsets[vect_offset >> 2][is_thumb];
u32 sctlr = vcpu_cp15(vcpu, c1_SCTLR);
cpsr = mode | COMPAT_PSR_I_BIT;
--
2.10.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort
2017-10-17 14:23 [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort Dongjiu Geng
@ 2017-10-18 10:09 ` Christoffer Dall
2017-10-18 10:19 ` Marc Zyngier
1 sibling, 0 replies; 4+ messages in thread
From: Christoffer Dall @ 2017-10-18 10:09 UTC (permalink / raw)
To: Dongjiu Geng
Cc: will.deacon, christoffer.dall, marc.zyngier, rkrcmar,
catalin.marinas, linux-arm-kernel, kvmarm, kvm, linux-kernel,
zhanghaibin7
On Tue, Oct 17, 2017 at 10:23:49PM +0800, Dongjiu Geng wrote:
> When a exception is trapped to EL2, hardware uses ELR_ELx to hold
> the current fault instruction address. If KVM wants to inject a
> abort to 32 bit guest, it needs to set the LR register for the
> guest to emulate this abort happened in the guest. Because ARM32
> architecture is pipelined execution, so the LR value has an offset to
> the fault instruction address.
>
> The offsets applied to Link value for exceptions as shown below,
> which should be added for the ARM32 link register(LR).
>
> Table taken from ARMv8 ARM DDI0487B-B, table G1-10:
> Exception Offset, for PE state of:
> A32 T32
> Undefined Instruction +4 +2
> Prefetch Abort +4 +4
> Data Abort +8 +8
> IRQ or FIQ +4 +4
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> Tested-by: Haibin Zhang <zhanghaibin7@huawei.com>
>
> ---
> Have tested in both arm32 and arm64
>
> For example, in the arm64 platform, to the undefined instruction injection:
>
> 1. Guest OS call SMC(Secure Monitor Call) instruction in the address
> 0xc025405c, then Guest traps to hypervisor
>
> c0254050: e59d5028 ldr r5, [sp, #40] ; 0x28
> c0254054: e3a03001 mov r3, #1
> c0254058: e1a01003 mov r1, r3
> c025405c: e1600070 smc 0
> c0254060: e30a0270 movw r0, #41584 ; 0xa270
> c0254064: e34c00bf movt r0, #49343 ; 0xc0bf
>
> 2. KVM injects undefined abort to guest
> 3. We will find the fault PC is 0xc0254058, not 0xc025405c.
>
> [ 12.348072] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> [ 12.349786] Modules linked in:
> [ 12.350563] CPU: 1 PID: 71 Comm: cat Not tainted 4.1.0-dirty #25
> [ 12.352061] Hardware name: Generic DT based system
> [ 12.353275] task: d9d08000 ti: d9cfe000 task.ti: d9cfe000
> [ 12.354637] PC is at proc_dointvec+0x20/0x60
> [ 12.355717] LR is at proc_sys_call_handler+0xb0/0xc4
> [ 12.356972] pc : [<c0254058>] lr : [<c035699c>] psr: a0060013
> [ 12.356972] sp : d9cffe90 ip : c0254038 fp : 00000001
> [ 12.359824] r10: d9cfff80 r9 : 00000004 r8 : 00000000
> [ 12.361132] r7 : bec21cb0 r6 : d9cffec4 r5 : d9cfff80 r4 : c0e82de0
> [ 12.362766] r3 : 00000001 r2 : bec21cb0 r1 : 00000001 r0 : c0e82de0
> [ 12.364400] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 12.366183] Control: 10c5383d Table: 59d3406a DAC: 00000015
> [ 12.367623] Process cat (pid: 71, stack limit = 0xd9cfe220)
>
> 4. After correct the LR register, it will have right value
>
> [ 125.763370] Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
> [ 125.767010] Modules linked in:
> [ 125.768472] CPU: 1 PID: 74 Comm: cat Tainted: G D 4.1.0-dirty #25
> [ 125.771854] Hardware name: Generic DT based system
> [ 125.774053] task: db0bb900 ti: d9d10000 task.ti: d9d10000
> [ 125.776821] PC is at proc_dointvec+0x24/0x60
> [ 125.778919] LR is at proc_sys_call_handler+0xb0/0xc4
> [ 125.781269] pc : [<c025405c>] lr : [<c035699c>] psr: a0060013
> [ 125.781269] sp : d9d11e90 ip : c0254038 fp : 00000001
> [ 125.786581] r10: d9d11f80 r9 : 00000004 r8 : 00000000
> [ 125.789673] r7 : be92ccb0 r6 : d9d11ec4 r5 : d9d11f80 r4 : c0e82de0
> [ 125.792828] r3 : 00000001 r2 : be92ccb0 r1 : 00000001 r0 : c0e82de0
> [ 125.795890] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>
> For other exception injection, such as Data/Prefetch abort, also needs to correct
> ---
> arch/arm/kvm/emulate.c | 4 ++--
> arch/arm64/kvm/inject_fault.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> index 0064b86..2419328 100644
> --- a/arch/arm/kvm/emulate.c
> +++ b/arch/arm/kvm/emulate.c
> @@ -227,7 +227,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> u32 return_offset = (is_thumb) ? 2 : 4;
>
> kvm_update_psr(vcpu, UND_MODE);
> - *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
> + *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
>
> /* Branch to exception vector */
> *vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
> @@ -242,7 +242,7 @@ static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
> unsigned long cpsr = *vcpu_cpsr(vcpu);
> bool is_thumb = (cpsr & PSR_T_BIT);
> u32 vect_offset;
> - u32 return_offset = (is_thumb) ? 4 : 0;
> + u32 return_offset = (is_pabt) ? 4 : 8;
> bool is_lpae;
>
> kvm_update_psr(vcpu, ABT_MODE);
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index da6a8cf..0416f18 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -33,12 +33,26 @@
> #define LOWER_EL_AArch64_VECTOR 0x400
> #define LOWER_EL_AArch32_VECTOR 0x600
>
> +/*
> + * Table taken from ARMv8 ARM DDI0487B-B, table G1-10.
> + */
> +static u8 return_offsets[8][2] = {
> + [0] = { 0, 0 }, /* Reset, unused */
> + [1] = { 4, 2 }, /* Undefined */
> + [2] = { 0, 0 }, /* SVC, unused */
> + [3] = { 4, 4 }, /* Prefetch abort */
> + [4] = { 8, 8 }, /* Data abort */
> + [5] = { 0, 0 }, /* HVC, unused */
> + [6] = { 4, 4 }, /* IRQ, unused */
> + [7] = { 4, 4 }, /* FIQ, unused */
> +};
> +
> static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
> {
> unsigned long cpsr;
> unsigned long new_spsr_value = *vcpu_cpsr(vcpu);
> bool is_thumb = (new_spsr_value & COMPAT_PSR_T_BIT);
> - u32 return_offset = (is_thumb) ? 4 : 0;
> + u32 return_offset = return_offsets[vect_offset >> 2][is_thumb];
> u32 sctlr = vcpu_cp15(vcpu, c1_SCTLR);
>
> cpsr = mode | COMPAT_PSR_I_BIT;
> --
> 2.10.1
>
Applied, thanks.
-Christoffer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort
2017-10-17 14:23 [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort Dongjiu Geng
2017-10-18 10:09 ` Christoffer Dall
@ 2017-10-18 10:19 ` Marc Zyngier
2017-10-18 11:39 ` Christoffer Dall
1 sibling, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2017-10-18 10:19 UTC (permalink / raw)
To: Dongjiu Geng, will.deacon, christoffer.dall, rkrcmar,
catalin.marinas, linux-arm-kernel, kvmarm, kvm, linux-kernel,
zhanghaibin7
On 17/10/17 15:23, Dongjiu Geng wrote:
> When a exception is trapped to EL2, hardware uses ELR_ELx to hold
> the current fault instruction address. If KVM wants to inject a
> abort to 32 bit guest, it needs to set the LR register for the
> guest to emulate this abort happened in the guest. Because ARM32
> architecture is pipelined execution, so the LR value has an offset to
> the fault instruction address.
>
> The offsets applied to Link value for exceptions as shown below,
> which should be added for the ARM32 link register(LR).
>
> Table taken from ARMv8 ARM DDI0487B-B, table G1-10:
> Exception Offset, for PE state of:
> A32 T32
> Undefined Instruction +4 +2
> Prefetch Abort +4 +4
> Data Abort +8 +8
> IRQ or FIQ +4 +4
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> Tested-by: Haibin Zhang <zhanghaibin7@huawei.com>
Cc: stable@vger.kernel.org
>
> ---
> Have tested in both arm32 and arm64
>
> For example, in the arm64 platform, to the undefined instruction injection:
>
> 1. Guest OS call SMC(Secure Monitor Call) instruction in the address
> 0xc025405c, then Guest traps to hypervisor
>
> c0254050: e59d5028 ldr r5, [sp, #40] ; 0x28
> c0254054: e3a03001 mov r3, #1
> c0254058: e1a01003 mov r1, r3
> c025405c: e1600070 smc 0
> c0254060: e30a0270 movw r0, #41584 ; 0xa270
> c0254064: e34c00bf movt r0, #49343 ; 0xc0bf
>
> 2. KVM injects undefined abort to guest
> 3. We will find the fault PC is 0xc0254058, not 0xc025405c.
>
> [ 12.348072] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> [ 12.349786] Modules linked in:
> [ 12.350563] CPU: 1 PID: 71 Comm: cat Not tainted 4.1.0-dirty #25
> [ 12.352061] Hardware name: Generic DT based system
> [ 12.353275] task: d9d08000 ti: d9cfe000 task.ti: d9cfe000
> [ 12.354637] PC is at proc_dointvec+0x20/0x60
> [ 12.355717] LR is at proc_sys_call_handler+0xb0/0xc4
> [ 12.356972] pc : [<c0254058>] lr : [<c035699c>] psr: a0060013
> [ 12.356972] sp : d9cffe90 ip : c0254038 fp : 00000001
> [ 12.359824] r10: d9cfff80 r9 : 00000004 r8 : 00000000
> [ 12.361132] r7 : bec21cb0 r6 : d9cffec4 r5 : d9cfff80 r4 : c0e82de0
> [ 12.362766] r3 : 00000001 r2 : bec21cb0 r1 : 00000001 r0 : c0e82de0
> [ 12.364400] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 12.366183] Control: 10c5383d Table: 59d3406a DAC: 00000015
> [ 12.367623] Process cat (pid: 71, stack limit = 0xd9cfe220)
>
> 4. After correct the LR register, it will have right value
>
> [ 125.763370] Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
> [ 125.767010] Modules linked in:
> [ 125.768472] CPU: 1 PID: 74 Comm: cat Tainted: G D 4.1.0-dirty #25
> [ 125.771854] Hardware name: Generic DT based system
> [ 125.774053] task: db0bb900 ti: d9d10000 task.ti: d9d10000
> [ 125.776821] PC is at proc_dointvec+0x24/0x60
> [ 125.778919] LR is at proc_sys_call_handler+0xb0/0xc4
> [ 125.781269] pc : [<c025405c>] lr : [<c035699c>] psr: a0060013
> [ 125.781269] sp : d9d11e90 ip : c0254038 fp : 00000001
> [ 125.786581] r10: d9d11f80 r9 : 00000004 r8 : 00000000
> [ 125.789673] r7 : be92ccb0 r6 : d9d11ec4 r5 : d9d11f80 r4 : c0e82de0
> [ 125.792828] r3 : 00000001 r2 : be92ccb0 r1 : 00000001 r0 : c0e82de0
> [ 125.795890] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>
> For other exception injection, such as Data/Prefetch abort, also needs to correct
> ---
> arch/arm/kvm/emulate.c | 4 ++--
> arch/arm64/kvm/inject_fault.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> index 0064b86..2419328 100644
> --- a/arch/arm/kvm/emulate.c
> +++ b/arch/arm/kvm/emulate.c
> @@ -227,7 +227,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> u32 return_offset = (is_thumb) ? 2 : 4;
>
> kvm_update_psr(vcpu, UND_MODE);
> - *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
> + *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
>
> /* Branch to exception vector */
> *vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
> @@ -242,7 +242,7 @@ static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
> unsigned long cpsr = *vcpu_cpsr(vcpu);
> bool is_thumb = (cpsr & PSR_T_BIT);
> u32 vect_offset;
> - u32 return_offset = (is_thumb) ? 4 : 0;
> + u32 return_offset = (is_pabt) ? 4 : 8;
> bool is_lpae;
>
> kvm_update_psr(vcpu, ABT_MODE);
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index da6a8cf..0416f18 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -33,12 +33,26 @@
> #define LOWER_EL_AArch64_VECTOR 0x400
> #define LOWER_EL_AArch32_VECTOR 0x600
>
> +/*
> + * Table taken from ARMv8 ARM DDI0487B-B, table G1-10.
> + */
> +static u8 return_offsets[8][2] = {
static const
> + [0] = { 0, 0 }, /* Reset, unused */
> + [1] = { 4, 2 }, /* Undefined */
> + [2] = { 0, 0 }, /* SVC, unused */
> + [3] = { 4, 4 }, /* Prefetch abort */
> + [4] = { 8, 8 }, /* Data abort */
> + [5] = { 0, 0 }, /* HVC, unused */
> + [6] = { 4, 4 }, /* IRQ, unused */
> + [7] = { 4, 4 }, /* FIQ, unused */
> +};
> +
> static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
> {
> unsigned long cpsr;
> unsigned long new_spsr_value = *vcpu_cpsr(vcpu);
> bool is_thumb = (new_spsr_value & COMPAT_PSR_T_BIT);
> - u32 return_offset = (is_thumb) ? 4 : 0;
> + u32 return_offset = return_offsets[vect_offset >> 2][is_thumb];
> u32 sctlr = vcpu_cp15(vcpu, c1_SCTLR);
>
> cpsr = mode | COMPAT_PSR_I_BIT;
>
Otherwise:
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
I'll shortly post a patch moving most of the 32bit fault injection to
virt/kvm/arm/aarch32.c so that we don't have two subtly different copies
of the same code...
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort
2017-10-18 10:19 ` Marc Zyngier
@ 2017-10-18 11:39 ` Christoffer Dall
0 siblings, 0 replies; 4+ messages in thread
From: Christoffer Dall @ 2017-10-18 11:39 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, catalin.marinas, will.deacon, linux-kernel, Dongjiu Geng,
kvmarm, linux-arm-kernel, zhanghaibin7
On Wed, Oct 18, 2017 at 11:19:30AM +0100, Marc Zyngier wrote:
> On 17/10/17 15:23, Dongjiu Geng wrote:
> > When a exception is trapped to EL2, hardware uses ELR_ELx to hold
> > the current fault instruction address. If KVM wants to inject a
> > abort to 32 bit guest, it needs to set the LR register for the
> > guest to emulate this abort happened in the guest. Because ARM32
> > architecture is pipelined execution, so the LR value has an offset to
> > the fault instruction address.
> >
> > The offsets applied to Link value for exceptions as shown below,
> > which should be added for the ARM32 link register(LR).
> >
> > Table taken from ARMv8 ARM DDI0487B-B, table G1-10:
> > Exception Offset, for PE state of:
> > A32 T32
> > Undefined Instruction +4 +2
> > Prefetch Abort +4 +4
> > Data Abort +8 +8
> > IRQ or FIQ +4 +4
> >
> > Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> > Tested-by: Haibin Zhang <zhanghaibin7@huawei.com>
>
> Cc: stable@vger.kernel.org
>
> >
> > ---
> > Have tested in both arm32 and arm64
> >
> > For example, in the arm64 platform, to the undefined instruction injection:
> >
> > 1. Guest OS call SMC(Secure Monitor Call) instruction in the address
> > 0xc025405c, then Guest traps to hypervisor
> >
> > c0254050: e59d5028 ldr r5, [sp, #40] ; 0x28
> > c0254054: e3a03001 mov r3, #1
> > c0254058: e1a01003 mov r1, r3
> > c025405c: e1600070 smc 0
> > c0254060: e30a0270 movw r0, #41584 ; 0xa270
> > c0254064: e34c00bf movt r0, #49343 ; 0xc0bf
> >
> > 2. KVM injects undefined abort to guest
> > 3. We will find the fault PC is 0xc0254058, not 0xc025405c.
> >
> > [ 12.348072] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> > [ 12.349786] Modules linked in:
> > [ 12.350563] CPU: 1 PID: 71 Comm: cat Not tainted 4.1.0-dirty #25
> > [ 12.352061] Hardware name: Generic DT based system
> > [ 12.353275] task: d9d08000 ti: d9cfe000 task.ti: d9cfe000
> > [ 12.354637] PC is at proc_dointvec+0x20/0x60
> > [ 12.355717] LR is at proc_sys_call_handler+0xb0/0xc4
> > [ 12.356972] pc : [<c0254058>] lr : [<c035699c>] psr: a0060013
> > [ 12.356972] sp : d9cffe90 ip : c0254038 fp : 00000001
> > [ 12.359824] r10: d9cfff80 r9 : 00000004 r8 : 00000000
> > [ 12.361132] r7 : bec21cb0 r6 : d9cffec4 r5 : d9cfff80 r4 : c0e82de0
> > [ 12.362766] r3 : 00000001 r2 : bec21cb0 r1 : 00000001 r0 : c0e82de0
> > [ 12.364400] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > [ 12.366183] Control: 10c5383d Table: 59d3406a DAC: 00000015
> > [ 12.367623] Process cat (pid: 71, stack limit = 0xd9cfe220)
> >
> > 4. After correct the LR register, it will have right value
> >
> > [ 125.763370] Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
> > [ 125.767010] Modules linked in:
> > [ 125.768472] CPU: 1 PID: 74 Comm: cat Tainted: G D 4.1.0-dirty #25
> > [ 125.771854] Hardware name: Generic DT based system
> > [ 125.774053] task: db0bb900 ti: d9d10000 task.ti: d9d10000
> > [ 125.776821] PC is at proc_dointvec+0x24/0x60
> > [ 125.778919] LR is at proc_sys_call_handler+0xb0/0xc4
> > [ 125.781269] pc : [<c025405c>] lr : [<c035699c>] psr: a0060013
> > [ 125.781269] sp : d9d11e90 ip : c0254038 fp : 00000001
> > [ 125.786581] r10: d9d11f80 r9 : 00000004 r8 : 00000000
> > [ 125.789673] r7 : be92ccb0 r6 : d9d11ec4 r5 : d9d11f80 r4 : c0e82de0
> > [ 125.792828] r3 : 00000001 r2 : be92ccb0 r1 : 00000001 r0 : c0e82de0
> > [ 125.795890] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> >
> > For other exception injection, such as Data/Prefetch abort, also needs to correct
> > ---
> > arch/arm/kvm/emulate.c | 4 ++--
> > arch/arm64/kvm/inject_fault.c | 16 +++++++++++++++-
> > 2 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> > index 0064b86..2419328 100644
> > --- a/arch/arm/kvm/emulate.c
> > +++ b/arch/arm/kvm/emulate.c
> > @@ -227,7 +227,7 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> > u32 return_offset = (is_thumb) ? 2 : 4;
> >
> > kvm_update_psr(vcpu, UND_MODE);
> > - *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) - return_offset;
> > + *vcpu_reg(vcpu, 14) = *vcpu_pc(vcpu) + return_offset;
> >
> > /* Branch to exception vector */
> > *vcpu_pc(vcpu) = exc_vector_base(vcpu) + vect_offset;
> > @@ -242,7 +242,7 @@ static void inject_abt(struct kvm_vcpu *vcpu, bool is_pabt, unsigned long addr)
> > unsigned long cpsr = *vcpu_cpsr(vcpu);
> > bool is_thumb = (cpsr & PSR_T_BIT);
> > u32 vect_offset;
> > - u32 return_offset = (is_thumb) ? 4 : 0;
> > + u32 return_offset = (is_pabt) ? 4 : 8;
> > bool is_lpae;
> >
> > kvm_update_psr(vcpu, ABT_MODE);
> > diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> > index da6a8cf..0416f18 100644
> > --- a/arch/arm64/kvm/inject_fault.c
> > +++ b/arch/arm64/kvm/inject_fault.c
> > @@ -33,12 +33,26 @@
> > #define LOWER_EL_AArch64_VECTOR 0x400
> > #define LOWER_EL_AArch32_VECTOR 0x600
> >
> > +/*
> > + * Table taken from ARMv8 ARM DDI0487B-B, table G1-10.
> > + */
> > +static u8 return_offsets[8][2] = {
>
> static const
>
> > + [0] = { 0, 0 }, /* Reset, unused */
> > + [1] = { 4, 2 }, /* Undefined */
> > + [2] = { 0, 0 }, /* SVC, unused */
> > + [3] = { 4, 4 }, /* Prefetch abort */
> > + [4] = { 8, 8 }, /* Data abort */
> > + [5] = { 0, 0 }, /* HVC, unused */
> > + [6] = { 4, 4 }, /* IRQ, unused */
> > + [7] = { 4, 4 }, /* FIQ, unused */
> > +};
> > +
> > static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
> > {
> > unsigned long cpsr;
> > unsigned long new_spsr_value = *vcpu_cpsr(vcpu);
> > bool is_thumb = (new_spsr_value & COMPAT_PSR_T_BIT);
> > - u32 return_offset = (is_thumb) ? 4 : 0;
> > + u32 return_offset = return_offsets[vect_offset >> 2][is_thumb];
> > u32 sctlr = vcpu_cp15(vcpu, c1_SCTLR);
> >
> > cpsr = mode | COMPAT_PSR_I_BIT;
> >
>
> Otherwise:
>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>
I fixed up the const and applied cc stable.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-18 11:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-17 14:23 [PATCH RESEND v2] arm/arm64: KVM: set right LR register value for 32 bit guest when inject abort Dongjiu Geng
2017-10-18 10:09 ` Christoffer Dall
2017-10-18 10:19 ` Marc Zyngier
2017-10-18 11:39 ` Christoffer Dall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox