Hi Andrew,
On Wed, 15 Jul 2020 19:44:08 +0100,
Andrew Scull <ascull@xxxxxxxxxx> wrote:
>
> When exiting a guest, just check whether there is an SError pending and
> set the bit in the exit code. The fixup then initiates the ceremony
> should it be required.
>
> The separation allows for easier choices to be made as to whether the
> demonic consultation should proceed.
Such as?
>
> Signed-off-by: Andrew Scull <ascull@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_hyp.h | 2 ++
> arch/arm64/kvm/hyp/entry.S | 27 +++++++++++++++++--------
> arch/arm64/kvm/hyp/hyp-entry.S | 1 -
> arch/arm64/kvm/hyp/include/hyp/switch.h | 4 ++++
> 4 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 07745d9c49fc..50a774812761 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -91,6 +91,8 @@ void deactivate_traps_vhe_put(void);
>
> u64 __guest_enter(struct kvm_vcpu *vcpu, struct kvm_cpu_context *host_ctxt);
>
> +void __vaxorcize_serror(void);
I think a VAXorsist reference in the comments is plenty. The code can
definitely stay architectural. Something like "__handle_SEI()" should
be good. I'm not *that* fun.
> +
> void __noreturn hyp_panic(struct kvm_cpu_context *host_ctxt);
> #ifdef __KVM_NVHE_HYPERVISOR__
> void __noreturn __hyp_do_panic(unsigned long, ...);
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 6a641fcff4f7..dc4e3e7e7407 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -174,18 +174,31 @@ alternative_if ARM64_HAS_RAS_EXTN
> mrs_s x2, SYS_DISR_EL1
> str x2, [x1, #(VCPU_FAULT_DISR - VCPU_CONTEXT)]
> cbz x2, 1f
> - msr_s SYS_DISR_EL1, xzr
> orr x0, x0, #(1<<ARM_EXIT_WITH_SERROR_BIT)
> -1: ret
> + nop
> +1:
> alternative_else
> dsb sy // Synchronize against in-flight ld/st
> isb // Prevent an early read of side-effect free ISR
> mrs x2, isr_el1
> - tbnz x2, #8, 2f // ISR_EL1.A
> - ret
> - nop
> + tbz x2, #8, 2f // ISR_EL1.A
> + orr x0, x0, #(1<<ARM_EXIT_WITH_SERROR_BIT)
> 2:
> alternative_endif
> +
> + ret
> +SYM_FUNC_END(__guest_enter)
> +
> +/*
> + * void __vaxorcize_serror(void);
> + */
> +SYM_FUNC_START(__vaxorcize_serror)
> +
> +alternative_if ARM64_HAS_RAS_EXTN
> + msr_s SYS_DISR_EL1, xzr
> + ret
> +alternative_else_nop_endif
> +
> // We know we have a pending asynchronous abort, now is the
> // time to flush it out. From your VAXorcist book, page 666:
> // "Threaten me not, oh Evil one! For I speak with
> @@ -193,7 +206,6 @@ alternative_endif
> mrs x2, elr_el2
> mrs x3, esr_el2
> mrs x4, spsr_el2
> - mov x5, x0
>
> msr daifclr, #4 // Unmask aborts
>
> @@ -217,6 +229,5 @@ abort_guest_exit_end:
> msr elr_el2, x2
> msr esr_el2, x3
> msr spsr_el2, x4
> - orr x0, x0, x5
> 1: ret
> -SYM_FUNC_END(__guest_enter)
> +SYM_FUNC_END(__vaxorcize_serror)
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> index e727bee8e110..c441aabb8ab0 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -177,7 +177,6 @@ el2_error:
> adr x1, abort_guest_exit_end
> ccmp x0, x1, #4, ne
> b.ne __hyp_panic
> - mov x0, #(1 << ARM_EXIT_WITH_SERROR_BIT)
> eret
> sb
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 0511af14dc81..14a774d1a35a 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -405,6 +405,10 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
> */
> static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> {
> + /* Flush guest SErrors. */
> + if (ARM_SERROR_PENDING(*exit_code))
> + __vaxorcize_serror();
> +
> if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
> vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
>
> --
> 2.27.0.389.gc38d7665816-goog
>
>
I'm not against this patch, but I wonder whether it actually helps
with anything. It spreads the handling across multiple paths, making
it harder to read. Could you explain the rational beyond "it's
easier"?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
