KVM: arm64: Decode basic HYP fault information

PATCH In review Pavel Fedin 2 participants 5 messages Aug 11, 2015 – Aug 31, 2015

Subsystems (3)

KVM arm64 KVM arm64

lore

Share & tools

lore

Pavel Fedin

author

2015-08-11 07:34 UTC #0 lore raw

PATCH 1/1 KVM: arm64: Decode basic HYP fault information

Print exception vector name, exception class and PC translated to EL1 virtual address. Significantly aids debugging HYP crashes without special means like JTAG.

Signed-off-by Pavel Fedin <[email protected]>

---
 arch/arm64/kvm/handle_exit.c | 30 +++++++++++++++++++++++++++++
 arch/arm64/kvm/hyp.S         | 46 +++++++++++++++++---------------------------
 2 files changed, 48 insertions(+), 28 deletions(-)

arch/arm64/kvm/handle_exit.c +30 -0

index 29b184a..4d70d64 100644
@@ -136,3 +136,33 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		return 0;
 	}
 }
+
+static const char *const hyp_faults[] = {
+	"EL2t Synchronous",
+	"EL2t IRQ",
+	"EL2t FIQ",
+	"EL2t Error",
+	"EL2h Synchronous",
+	"EL2h IRQ",
+	"EL2h FIQ",
+	"EL2h Error",
+	"EL1 Synchronous",
+	"EL1 IRQ",
+	"EL1 FIQ",
+	"EL1 Error"
+};
+
+void kvm_hyp_panic(unsigned long vector, unsigned int spsr, unsigned long pc,
+		   unsigned int esr, unsigned long far, unsigned long hpfar,
+		   unsigned long par, struct kvm_vcpu *vcpu)
+{
+	pr_emerg("Unhandled HYP exception %s on VCPU %p\n",
+		hyp_faults[vector], vcpu);
+	pr_emerg("PC : %016lx SPSR : %08x         ESR: %08x\n", pc, spsr, esr);
+	pr_emerg("FAR: %016lx HPFAR: %016lx PAR: %016lx\n", far, hpfar, par);
+
+	pr_emerg("Exception class: %02x Translated PC: %016lx\n",
+		esr >> ESR_ELx_EC_SHIFT, pc - HYP_PAGE_OFFSET + PAGE_OFFSET);
+
+	panic("HYP panic");
+}

arch/arm64/kvm/hyp.S +18 ~~-28~~

77 lines not on this page · raw message

Signature · 7 lines

2.4.4


Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Christoffer Dall

2015-08-30 17:43 UTC #1 lore raw

Quoted 3 lines · Pavel Fedin

Print exception vector name, exception class and PC translated to EL1 virtual address. Significantly aids debugging HYP crashes without special means like JTAG.

my overall concern with this patch is that it adds complexity to an already really bad situation, and potentially increases the likelihood of not seeing any debug info at all.

do you encounter this kind of panic a lot? I haven't experienced a great need for more hyp debugging help lately...

Quoted 68 lines

Signed-off-by Pavel Fedin <[email protected]>

---
 arch/arm64/kvm/handle_exit.c | 30 +++++++++++++++++++++++++++++
 arch/arm64/kvm/hyp.S         | 46 +++++++++++++++++---------------------------
 2 files changed, 48 insertions(+), 28 deletions(-)

arch/arm64/kvm/handle_exit.c +30 -0

index 29b184a..4d70d64 100644
@@ -136,3 +136,33 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		return 0;
 	}
 }
+
+static const char *const hyp_faults[] = {
+	"EL2t Synchronous",
+	"EL2t IRQ",
+	"EL2t FIQ",
+	"EL2t Error",
+	"EL2h Synchronous",
+	"EL2h IRQ",
+	"EL2h FIQ",
+	"EL2h Error",
+	"EL1 Synchronous",
+	"EL1 IRQ",
+	"EL1 FIQ",
+	"EL1 Error"
+};
+
+void kvm_hyp_panic(unsigned long vector, unsigned int spsr, unsigned long pc,
+		   unsigned int esr, unsigned long far, unsigned long hpfar,
+		   unsigned long par, struct kvm_vcpu *vcpu)
+{
+	pr_emerg("Unhandled HYP exception %s on VCPU %p\n",
+		hyp_faults[vector], vcpu);
+	pr_emerg("PC : %016lx SPSR : %08x         ESR: %08x\n", pc, spsr, esr);
+	pr_emerg("FAR: %016lx HPFAR: %016lx PAR: %016lx\n", far, hpfar, par);
+
+	pr_emerg("Exception class: %02x Translated PC: %016lx\n",
+		esr >> ESR_ELx_EC_SHIFT, pc - HYP_PAGE_OFFSET + PAGE_OFFSET);
+
+	panic("HYP panic");
+}

arch/arm64/kvm/hyp.S +3 -5

index c81eaaf..62785cd 100644
@@ -1060,13 +1060,11 @@ __kvm_hyp_panic:
 	ldr	x2, [x0, #VCPU_HOST_CONTEXT]
 	kern_hyp_va x2
 
+	mov	x0, lr
 	bl __restore_sysregs
+	mov	lr, x0
 
-1:	adr	x0, __hyp_panic_str
-	adr	x1, 2f
-	ldp	x2, x3, [x1]
-	sub	x0, x0, x2
-	add	x0, x0, x3
+1:	mov	x0, lr
 	mrs	x1, spsr_el2
 	mrs	x2, elr_el2
 	mrs	x3, esr_el2

@@ -1078,20 +1076,11 @@ __kvm_hyp_panic:
 	mov	lr, #(PSR_F_BIT | PSR_I_BIT | PSR_A_BIT | PSR_D_BIT |\

arch/arm64/kvm/hyp.S __kvm_hyp_panic: · line 1087

Quoted 10 lines

arch/arm64/kvm/hyp.S +1 -5

 		      PSR_MODE_EL1h)
 	msr	spsr_el2, lr
-	ldr	lr, =panic
+	ldr	lr, =kvm_hyp_panic
 	msr	elr_el2, lr
 	eret
-
-	.align	3
-2:	.quad	HYP_PAGE_OFFSET
-	.quad	PAGE_OFFSET

why can you get rid of this?

Quoted 62 lines

arch/arm64/kvm/hyp.S +14 ~~-18~~

 ENDPROC(__kvm_hyp_panic)
 
-__hyp_panic_str:
-	.ascii	"HYP panic:\nPS:%08x PC:%p ESR:%p\nFAR:%p HPFAR:%p PAR:%p\nVCPU:%p\n\0"
-
-	.align	2
-
 /*
  * u64 kvm_call_hyp(void *hypfn, ...);
  *

@@ -1115,26 +1104,27 @@ ENTRY(kvm_call_hyp)
 	ret
 ENDPROC(kvm_call_hyp)
 
-.macro invalid_vector	label, target
+.macro invalid_vector	label, N, target
 	.align	2
 \label:
+	mov lr, #\N
 	b \target
 ENDPROC(\label)
 .endm
 
 	/* None of these should ever happen */
-	invalid_vector	el2t_sync_invalid, __kvm_hyp_panic
-	invalid_vector	el2t_irq_invalid, __kvm_hyp_panic
-	invalid_vector	el2t_fiq_invalid, __kvm_hyp_panic
-	invalid_vector	el2t_error_invalid, __kvm_hyp_panic
-	invalid_vector	el2h_sync_invalid, __kvm_hyp_panic
-	invalid_vector	el2h_irq_invalid, __kvm_hyp_panic
-	invalid_vector	el2h_fiq_invalid, __kvm_hyp_panic
-	invalid_vector	el2h_error_invalid, __kvm_hyp_panic
-	invalid_vector	el1_sync_invalid, __kvm_hyp_panic
-	invalid_vector	el1_irq_invalid, __kvm_hyp_panic
-	invalid_vector	el1_fiq_invalid, __kvm_hyp_panic
-	invalid_vector	el1_error_invalid, __kvm_hyp_panic
+	invalid_vector	el2t_sync_invalid, 0, __kvm_hyp_panic
+	invalid_vector	el2t_irq_invalid, 1, __kvm_hyp_panic
+	invalid_vector	el2t_fiq_invalid, 2, __kvm_hyp_panic
+	invalid_vector	el2t_error_invalid, 3, __kvm_hyp_panic
+	invalid_vector	el2h_sync_invalid, 4, __kvm_hyp_panic
+	invalid_vector	el2h_irq_invalid, 5, __kvm_hyp_panic
+	invalid_vector	el2h_fiq_invalid, 6, __kvm_hyp_panic
+	invalid_vector	el2h_error_invalid, 7, __kvm_hyp_panic
+	invalid_vector	el1_sync_invalid, 8, __kvm_hyp_panic
+	invalid_vector	el1_irq_invalid, 9, __kvm_hyp_panic
+	invalid_vector	el1_fiq_invalid, 10, __kvm_hyp_panic
+	invalid_vector	el1_error_invalid, 11, __kvm_hyp_panic
 
 el1_sync:					// Guest trapped into EL2
 	push	x0, x1

Signature · 7 lines

2.4.4


Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Pavel Fedin

author

2015-08-31 06:33 UTC #2 lore raw

Hello!

Quoted 3 lines

my overall concern with this patch is that it adds complexity to an already really bad situation, and potentially increases the likelihood of not seeing any debug info at all.

Why? In this case we currently already drop into C code. I do the same, with some more useful
printout. What is fundamentally changed?

Quoted 2 lines

do you encounter this kind of panic a lot? I haven't experienced a great need for more hyp debugging help lately...

I met it once, and i had a stable way to reproduce it. At the end it appeared to be another HW
quirk, but, still, this patch helped a lot to find out what was wrong. The main missing information was vector number, and translated PC is also useful.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Christoffer Dall

2015-08-31 09:48 UTC #3 lore raw

Quoted 9 lines · Pavel Fedin

Hello!
Quoted 3 lines
my overall concern with this patch is that it adds complexity to an already really bad situation, and potentially increases the likelihood of not seeing any debug info at all.
Why? In this case we currently already drop into C code. I do the same, with some more useful
printout. What is fundamentally changed?

Fundamentally what's changed is that you now run more custom code in C code.

I'll think about it.

-Christoffer

Pavel Fedin

author

2015-08-31 09:59 UTC #4 lore raw

Hello!

Quoted 2 lines

Fundamentally what's changed is that you now run more custom code in C code.

Yes, but it doesn't do anything super-dupper, just decodes some values into a human-readable form.

and i forgot this note...

Quoted 6 lines

arch/arm64/kvm/hyp.S __kvm_hyp_panic: · line 1087
Quoted 4 lines
arch/arm64/kvm/hyp.S +0 -4
-
-	.align	3
-2:	.quad	HYP_PAGE_OFFSET
-	.quad	PAGE_OFFSET
why can you get rid of this?

Because i was used only to translate base address of text message, which moved to C code.

Quoted 1 line

I'll think about it.

Ok.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia