On 12/10/17 11:41, Christoffer Dall wrote: > VHE actually doesn't rely on clearing the VTTBR when returning to the > hsot kernel, and that is the current key mechanism of hyp_panic to
host > figure out how to attempt to return to a state good enough to print a > panic statement. > > Therefore, we split the hyp_panic function into two functions, a VHE and > a non-VHE, keeping the non-VHE version intact, but changing the VHE > behavior. > > The vttbr_el2 check on VHE doesn't really make that much sense, because > the only situation where we can get here on VHE is when the hypervisor > assembly code actually caleld into hyp_panic, which only happens when called > VBAR_EL2 has been set to the KVM exception vectors. On VHE, we can > always safely disable the traps and restore the host registers at this > point, so we simply do that unconditionally and call into the panic > function directly. > > Signed-off-by: Christoffer Dall <christoffer.d...@linaro.org> > --- > arch/arm64/kvm/hyp/switch.c | 45 > +++++++++++++++++++++++---------------------- > 1 file changed, 23 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index a0123ad..a50ddf3 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -394,10 +394,20 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu) > static const char __hyp_panic_string[] = "HYP panic:\nPS:%08llx PC:%016llx > ESR:%08llx\nFAR:%016llx HPFAR:%016llx PAR:%016llx\nVCPU:%p\n"; > > static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par, > - struct kvm_vcpu *vcpu) > + struct kvm_cpu_context > *__host_ctxt) > { > + struct kvm_vcpu *vcpu; > unsigned long str_va; > > + vcpu = __host_ctxt->__hyp_running_vcpu; > + > + if (read_sysreg(vttbr_el2)) { > + __timer_disable_traps(vcpu); > + __deactivate_traps(vcpu); > + __deactivate_vm(vcpu); > + __sysreg_restore_host_state(__host_ctxt); > + } > + > /* > * Force the panic string to be loaded from the literal pool, > * making sure it is a kernel address and not a PC-relative > @@ -411,40 +421,31 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, > u64 elr, u64 par, > read_sysreg(hpfar_el2), par, vcpu); > } > > -static void __hyp_text __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par, > - struct kvm_vcpu *vcpu) > +static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par, > + struct kvm_cpu_context *host_ctxt) > { > + struct kvm_vcpu *vcpu; > + vcpu = host_ctxt->__hyp_running_vcpu; > + > + __deactivate_traps_vhe(); Is there a reason why we can't just call __deactivate_traps(), rather than the VHE-specific subset? It doesn't really matter, as we're about to panic, but still... > + __sysreg_restore_host_state(host_ctxt); > + > panic(__hyp_panic_string, > spsr, elr, > read_sysreg_el2(esr), read_sysreg_el2(far), > read_sysreg(hpfar_el2), par, vcpu); > } > > -static hyp_alternate_select(__hyp_call_panic, > - __hyp_call_panic_nvhe, __hyp_call_panic_vhe, > - ARM64_HAS_VIRT_HOST_EXTN); > - > void __hyp_text __noreturn hyp_panic(struct kvm_cpu_context *__host_ctxt) > { > - struct kvm_vcpu *vcpu = NULL; > - > u64 spsr = read_sysreg_el2(spsr); > u64 elr = read_sysreg_el2(elr); > u64 par = read_sysreg(par_el1); > > - if (read_sysreg(vttbr_el2)) { > - struct kvm_cpu_context *host_ctxt; > - > - host_ctxt = __host_ctxt; > - vcpu = host_ctxt->__hyp_running_vcpu; > - __timer_disable_traps(vcpu); > - __deactivate_traps(vcpu); > - __deactivate_vm(vcpu); > - __sysreg_restore_host_state(host_ctxt); > - } > - > - /* Call panic for real */ > - __hyp_call_panic()(spsr, elr, par, vcpu); > + if (!has_vhe()) > + __hyp_call_panic_nvhe(spsr, elr, par, __host_ctxt); > + else > + __hyp_call_panic_vhe(spsr, elr, par, __host_ctxt); > > unreachable(); > } > Otherwise looks good. M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm