Skip to contentSkip to conversation

KVM: arm/arm64: Restore host physical timer access on panic

PATCH In review James Morse 3 participants 6 messages Apr 25, 2017 – May 15, 2017

Subsystems (3)
Arm (32-bit) arm64 KVM arm64
lore
Share & tools
lore

James Morse

author
#0 lore raw

cover KVM: arm/arm64: Restore host physical timer access on panic

Hi!

On arm64, with a single CPU when I trigger hyp_panic() with the guest registers loaded, I get two traces:

[ 8736.164022] Kernel panic - not syncing: HYP panic:
[ 8736.164022] PS:600002c9 PC:0000800000a841e8 ESR:96000046
[ 8736.164022] FAR:0000000000000880 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[ 8736.164022] VCPU:0000800974430000
[ 8736.164022]
[ 8736.187686] CPU: 0 PID: 2449 Comm: kvm-vcpu-0 Not tainted 4.11.0-rc7-00002-ga55c0ba01d81-dirty #7487
[ 8736.196869] Hardware name: ARM Juno development board (r1) (DT)
[ 8736.202822] Call trace:
[ 8736.205300] [<ffff000008089c80>] dump_backtrace+0x0/0x238
[ 8736.210736] [<ffff000008089ecc>] show_stack+0x14/0x20
[ 8736.215824] [<ffff00000847a274>] dump_stack+0xbc/0xf8
[ 8736.220912] [<ffff0000081e0e60>] panic+0x124/0x29c
[ 8736.225737] [<ffff0000081e0d3c>] panic+0x0/0x29c
[ 8736.230486] Kernel Offset: disabled
[ 8736.234024] Memory Limit: none
[ 8736.237127] ---[ end Kernel panic - not syncing: HYP panic:
[ 8736.237127] PS:600002c9 PC:0000800000a841e8 ESR:96000046
[ 8736.237127] FAR:0000000000000880 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[ 8736.237127] VCPU:0000800974430000
[ 8736.237127]
[ 8736.259771] Kernel panic - not syncing: HYP panic:
[ 8736.259771] PS:200002c9 PC:0000200000080000 ESR:86000004
[ 8736.259771] FAR:0000200000080000 HPFAR:0000000000800800 PAR:0000000000000000
[ 8736.259771] VCPU:0000800974430000
[ 8736.259771]
[ 8736.281678] CPU: 0 PID: 2449 Comm: kvm-vcpu-0 Not tainted 4.11.0-rc7-00002-ga55c0ba01d81-dirty #7487
[ 8736.290811] Hardware name: ARM Juno development board (r1) (DT)
[ 8736.296726] Call trace:
[ 8736.299185] [<ffff000008089c80>] dump_backtrace+0x0/0x238
[ 8736.304588] [<ffff000008089ecc>] show_stack+0x14/0x20
[ 8736.309642] [<ffff00000847a274>] dump_stack+0xbc/0xf8
[ 8736.314698] [<ffff0000081e0e60>] panic+0x124/0x29c
[ 8736.319495] [<ffff0000081e0d3c>] panic+0x0/0x29c
[ 8736.324120] Kernel Offset: disabled
[ 8736.327611] Memory Limit: none
[ 8736.330687] ---[ end Kernel panic - not syncing: HYP panic:
[ 8736.330687] PS:200002c9 PC:0000200000080000 ESR:86000004
[ 8736.330687] FAR:0000200000080000 HPFAR:0000000000800800 PAR:0000000000000000
[ 8736.330687] VCPU:0000800974430000
[ 8736.330687]

This is because the physical timer access is still trapped to EL2, causing a second __guest_exit(), this time without the host context on the stack.

Once I get 32bit hyp_panic() to restore the hosts banked registers, I get the same:

[  164.799341] Kernel panic - not syncing:
[  164.799341] HYP panic: FIQ   PC:40010778 CPSR:900001d3
[  164.826708] CPU: 0 PID: 1933 Comm: qemu-system-arm Not tainted 4.11.0-rc6+ #196
[  164.848813] Hardware name: ARM-Versatile Express
[  164.862798] [<c022fbf8>] (unwind_backtrace) from [<c0229e40>] (show_stack+0x18/0x1c)
[  164.886218] [<c0229e40>] (show_stack) from [<c0516590>] (dump_stack+0x94/0xa8)
[  164.908071] [<c0516590>] (dump_stack) from [<c030a77c>] (panic+0xe4/0x270)
[  164.928864] [<c030a77c>] (panic) from [<c0213998>] (kvm_arch_vcpu_ioctl_run+0x1e4/0x6ac)
[  164.953332] [<c0213998>] (kvm_arch_vcpu_ioctl_run) from [<c020a3a0>] (kvm_vcpu_ioctl+0x2e4/0x808)
[  164.980184] [<c020a3a0>] (kvm_vcpu_ioctl) from [<c03806c8>] (do_vfs_ioctl+0xa8/0x7bc)
[  165.003880] [<c03806c8>] (do_vfs_ioctl) from [<c0380e18>] (SyS_ioctl+0x3c/0x64)
[  165.026002] [<c0380e18>] (SyS_ioctl) from [<c02259e0>] (ret_fast_syscall+0x0/0x34)
[  165.048911] ---[ end Kernel panic - not syncing:
[  165.048911] HYP panic: FIQ   PC:40010778 CPSR:900001d3
[  165.078657] Kernel panic - not syncing:
[  165.078657] HYP panic: PABRT PC:ee526000 CPSR:600000da
[  165.106031] CPU: 0 PID: 1933 Comm: qemu-system-arm Not tainted 4.11.0-rc6+ #196
[  165.128126] Hardware name: ARM-Versatile Express
[  165.142072] [<c022fbf8>] (unwind_backtrace) from [<c0229e40>] (show_stack+0x18/0x1c)
[  165.165511] [<c0229e40>] (show_stack) from [<c0516590>] (dump_stack+0x94/0xa8)
[  165.187351] [<c0516590>] (dump_stack) from [<c030a77c>] (panic+0xe4/0x270)
[  165.208151] [<c030a77c>] (panic) from [<c0213998>] (kvm_arch_vcpu_ioctl_run+0x1e4/0x6ac)
[  165.232630] [<c0213998>] (kvm_arch_vcpu_ioctl_run) from [<c020a3a0>] (kvm_vcpu_ioctl+0x2e4/0x808)
[  165.259464] [<c020a3a0>] (kvm_vcpu_ioctl) from [<c03806c8>] (do_vfs_ioctl+0xa8/0x7bc)
[  165.283150] [<c03806c8>] (do_vfs_ioctl) from [<c0380e18>] (SyS_ioctl+0x3c/0x64)
[  165.305247] [<c0380e18>] (SyS_ioctl) from [<c02259e0>] (ret_fast_syscall+0x0/0x34)
[  165.328163] ---[ end Kernel panic - not syncing:
[  165.328163] HYP panic: PABRT PC:ee526000 CPSR:600000da

Thanks,

James Morse (2):
KVM: arm: Restore banked registers and physical timer access on

    hyp_panic()

KVM: arm64: Restore host physical timer access on hyp_panic()

 arch/arm/kvm/hyp/switch.c   | 2 ++
 arch/arm64/kvm/hyp/switch.c | 1 +
 2 files changed, 3 insertions(+)
Signature · 1 line
2.10.1
author
#1 lore raw

PATCH 1/2 KVM: arm: Restore banked registers and physical timer access on hyp_panic()

When KVM panics, it hurridly restores the host context and parachutes into the host's panic() code. This looks like it was copied from arm64, the 32bit KVM panic code needs to restore the host's banked registers too.

At some point panic() touches the physical timer/counter, this will trap back to HYP. If we're lucky, we panic again.

Add a __timer_save_state() call to KVMs hyp_panic() path, this saves the guest registers and disables the traps for the host.

Fixes c36b6db5f3e4 ("ARM: KVM: Add panic handling code")
Signed-off-by James Morse <[email protected]>
---
 arch/arm/kvm/hyp/switch.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/kvm/hyp/switch.c +2 -0
index 92678b7bd046..c8f15bb5c8b3 100644
@@ -235,8 +235,10 @@ void __hyp_text __noreturn __hyp_panic(int cause)
 
 		vcpu = (struct kvm_vcpu *)read_sysreg(HTPIDR);
 		host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+		__timer_save_state(vcpu);
 		__deactivate_traps(vcpu);
 		__deactivate_vm(vcpu);
+		__banked_restore_state(host_ctxt);
 		__sysreg_restore_state(host_ctxt);
 	}
Signature · 1 line
2.10.1
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] KVM: arm: Restore banked registers and physical timer access on hyp_panic()

Quoted 22 lines · James Morse

When KVM panics, it hurridly restores the host context and parachutes into the host's panic() code. This looks like it was copied from arm64, the 32bit KVM panic code needs to restore the host's banked registers too.

At some point panic() touches the physical timer/counter, this will trap back to HYP. If we're lucky, we panic again.

Add a __timer_save_state() call to KVMs hyp_panic() path, this saves the guest registers and disables the traps for the host.

Fixes c36b6db5f3e4 ("ARM: KVM: Add panic handling code")
Signed-off-by James Morse <[email protected]>
---
 arch/arm/kvm/hyp/switch.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/kvm/hyp/switch.c +0 -0
index 92678b7bd046..c8f15bb5c8b3 100644

arch/arm/kvm/hyp/switch.c void __hyp_text __noreturn __hyp_panic(int cause) · line 243

Quoted 9 lines
arch/arm/kvm/hyp/switch.c +2 -0
@@ -235,8 +235,10 @@ void __hyp_text __noreturn __hyp_panic(int cause)
 
 		vcpu = (struct kvm_vcpu *)read_sysreg(HTPIDR);
 		host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+		__timer_save_state(vcpu);
 		__deactivate_traps(vcpu);
 		__deactivate_vm(vcpu);
+		__banked_restore_state(host_ctxt);
 		__sysreg_restore_state(host_ctxt);
 	}
Reviewed-by Marc Zyngier <[email protected]>

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author
#3 lore raw

PATCH 2/2 KVM: arm64: Restore host physical timer access on hyp_panic()

In reply to James Morse

When KVM panics, it hurridly restores the host context and parachutes into the host's panic() code. At some point panic() touches the physical timer/counter. Unless we are an arm64 system with VHE, this traps back to EL2. If we're lucky, we panic again.

Add a __timer_save_state() call to KVMs hyp_panic() path, this saves the guest registers and disables the traps for the host.

Fixes 53fd5b6487e4 ("arm64: KVM: Add panic handling")
Signed-off-by James Morse <[email protected]>
---
 arch/arm64/kvm/hyp/switch.c | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/kvm/hyp/switch.c +1 -0
index aede1658aeda..e5f089de6526 100644
@@ -422,6 +422,7 @@ void __hyp_text __noreturn __hyp_panic(void)
 
 		vcpu = (struct kvm_vcpu *)read_sysreg(tpidr_el2);
 		host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+		__timer_save_state(vcpu);
 		__deactivate_traps(vcpu);
 		__deactivate_vm(vcpu);
 		__sysreg_restore_host_state(host_ctxt);
Signature · 1 line
2.10.1
Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] KVM: arm64: Restore host physical timer access on hyp_panic()

Quoted 1 line · James Morse

When KVM panics, it hurridly restores the host context and parachutes

hurriedly?

Quoted 19 lines

into the host's panic() code. At some point panic() touches the physical timer/counter. Unless we are an arm64 system with VHE, this traps back to EL2. If we're lucky, we panic again.

Add a __timer_save_state() call to KVMs hyp_panic() path, this saves the guest registers and disables the traps for the host.

Fixes 53fd5b6487e4 ("arm64: KVM: Add panic handling")
Signed-off-by James Morse <[email protected]>
---
 arch/arm64/kvm/hyp/switch.c | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/kvm/hyp/switch.c +0 -0
index aede1658aeda..e5f089de6526 100644

arch/arm64/kvm/hyp/switch.c void __hyp_text __noreturn __hyp_panic(void) · line 428

Quoted 7 lines
arch/arm64/kvm/hyp/switch.c +1 -0
@@ -422,6 +422,7 @@ void __hyp_text __noreturn __hyp_panic(void)
 
 		vcpu = (struct kvm_vcpu *)read_sysreg(tpidr_el2);
 		host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+		__timer_save_state(vcpu);
 		__deactivate_traps(vcpu);
 		__deactivate_vm(vcpu);
 		__sysreg_restore_host_state(host_ctxt);
Reviewed-by Marc Zyngier <[email protected]>

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Reviewed-by on the v1 series
#5 lore raw
In reply to James Morse

Hi James,

Quoted 6 lines · James Morse

Hi!

On arm64, with a single CPU when I trigger hyp_panic() with the guest registers loaded, I get two traces:

[ 8736.164022] Kernel panic - not syncing: HYP panic:

[...]

Quoted 7 lines

This is because the physical timer access is still trapped to EL2, causing a second __guest_exit(), this time without the host context on the stack.

Once I get 32bit hyp_panic() to restore the hosts banked registers, I get the same:

[  164.799341] Kernel panic - not syncing:

[...]

Thanks for these.

Reviewed-by Christoffer Dall <[email protected]>

And applied to kvmarm/queue.

-Christoffer