Hi Marc,
On Fri, Jan 21, 2022 at 6:42 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> Injecting an exception into a guest with non-VHE is risky business.
> Instead of writing in the shadow register for the switch code to
> restore it, we override the CPU register instead. Which gets
> overriden a few instructions later by said restore code.
I see that in __sysreg_restore_el1_state(), which as you said is
called after __vcpu_write_spsr().
> The result is that although the guest correctly gets the exception,
> it will return to the original context in some random state,
> depending on what was there the first place... Boo.
>
> Fix the issue by writing to the shadow register. The original code
> is absolutely fine on VHE, as the state is already loaded, and writing
> to the shadow register in that case would actually be a bug.
Which happens via kvm_vcpu_load_sysregs_vhe() calling
__sysreg_restore_el1_state() before __vcpu_write_spsr() in this case.
Reviewed-by: Fuad Tabba <tabba@xxxxxxxxxx>
Cheers,
/fuad
> Fixes: bb666c472ca2 ("KVM: arm64: Inject AArch64 exceptions from HYP")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/kvm/hyp/exception.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index 0418399e0a20..c5d009715402 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
> @@ -38,7 +38,10 @@ static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
>
> static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, u64 val)
> {
> - write_sysreg_el1(val, SYS_SPSR);
> + if (has_vhe())
> + write_sysreg_el1(val, SYS_SPSR);
> + else
> + __vcpu_sys_reg(vcpu, SPSR_EL1) = val;
> }
>
> static void __vcpu_write_spsr_abt(struct kvm_vcpu *vcpu, u64 val)
> --
> 2.34.1
>
