Re: [PATCH] target/arm: fix missing exception class

[Jamie Iles]

Hi Peter,

On Mon, May 24, 2021 at 10:41:58AM +0100, Peter Maydell wrote:
> On Mon, 24 May 2021 at 09:42, Jamie Iles <jamie@nuviainc.com> wrote:
> >
> > The DAIF and PAC checks used raise_exception_ra to raise an exception
> > and unwind CPU state but raise_exception_ra is currently designed for
> > handling data aborts as the syndrome is partially precomputed and
> > encoded in the TB and then merged in merge_syn_data_abort when handling
> > the data abort.  Using raise_exception_ra for DAIF and PAC checks
> > results in an empty syndrome being retrieved from data[2] in
> > restore_state_to_opc and setting ESR to 0.  This manifested as:
> >
> >   kvm [571]: Unknown exception class: esr: 0x000000 –
> >   Unknown/Uncategorized
> >
> > when launching a KVM guest when the host qemu used a CPU supporting
> > EL2+pointer authentication and enabling pointer authentication in the
> > guest.
> 
> raise_exception() and raise_exception_ra() are supposed to have
> the same semantics apart from one of them being passed a return
> address. So perhaps we should look at trying to fix this by
> making raise_exception_ra() not first carefully set and then
> very opaquely unconditionally trash env->exception.syndrome...

The simplest fix for this would be the patch below and that is 
consistent with the TLB fault handling code where we first restore state 
then raise the exception, is this the sort of thing you were thinking 
of?

Thanks,

Jamie

diff --git i/target/arm/op_helper.c w/target/arm/op_helper.c
index efcb60099277..0b85403cb9f4 100644
--- i/target/arm/op_helper.c
+++ w/target/arm/op_helper.c
@@ -63,8 +63,11 @@ void raise_exception(CPUARMState *env, uint32_t excp,
 void raise_exception_ra(CPUARMState *env, uint32_t excp, uint32_t syndrome,
                         uint32_t target_el, uintptr_t ra)
 {
-    CPUState *cs = do_raise_exception(env, excp, syndrome, target_el);
-    cpu_loop_exit_restore(cs, ra);
+    CPUState *cs = env_cpu(env);
+
+    cpu_restore_state(cs, ra, true);
+    do_raise_exception(env, excp, syndrome, target_el);
+    cpu_loop_exit(cs);
 }
 
 uint64_t HELPER(neon_tbl)(CPUARMState *env, uint32_t desc,