[PATCH 0/2] KVM: arm/arm64: Restore host physical timer access on panic

[PATCH 0/2] KVM: arm/arm64: Restore host physical timer access on panic

[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:
[ 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(+)

-- 
2.10.1

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

[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 <james.morse@arm.com>
---
 arch/arm/kvm/hyp/switch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
index 92678b7bd046..c8f15bb5c8b3 100644
--- a/arch/arm/kvm/hyp/switch.c
+++ b/arch/arm/kvm/hyp/switch.c
@@ -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);
 	}
 
-- 
2.10.1

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

[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 <james.morse@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index aede1658aeda..e5f089de6526 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -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);
-- 
2.10.1

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

[Marc Zyngier]

On 25/04/17 18:02, James Morse wrote:
> 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 <james.morse@arm.com>
> ---
>  arch/arm/kvm/hyp/switch.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
> index 92678b7bd046..c8f15bb5c8b3 100644
> --- a/arch/arm/kvm/hyp/switch.c
> +++ b/arch/arm/kvm/hyp/switch.c
> @@ -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 <marc.zyngier@arm.com>

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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

[Marc Zyngier]

On 25/04/17 18:02, James Morse wrote:
> When KVM panics, it hurridly restores the host context and parachutes

                      hurriedly?

> 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 <james.morse@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index aede1658aeda..e5f089de6526 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -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 <marc.zyngier@arm.com>

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/2] KVM: arm/arm64: Restore host physical timer access on panic

[Christoffer Dall]

Hi James,

On Tue, Apr 25, 2017 at 06:02:43PM +0100, James Morse wrote:
> 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:

[...]

> 
> 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 <cdall@linaro.org>

And applied to kvmarm/queue.

-Christoffer