Re: [Qemu-devel] [[PATCH] 1/7] target-arm: Add exception target el infrastructure

[Peter Maydell]

On 27 March 2015 at 19:10, Greg Bellows <address@hidden> wrote:
> Add a CPU state exception target EL field that will be used for communicating
> the EL to which an exception should be routed.
>
> Add a target EL argument to the generic exception helper for callers to 
> specify
> the EL to which the exception should be routed.  Extended the helper to set
> the newly added CPU state exception target el.  Updated calls to helpers to
> include target EL, minimally the current el, which gets upgraded as needed.
>
> Signed-off-by: Greg Bellows <address@hidden>

This is generally OK, but I don't like the way we directly use
s->current_el as the target EL in many places:

(1) putting the MAX(target_el, 1) in gen_exception() feels
like the wrong place.
(2) if we're executing with a 32-bit EL3 and we're in Secure
EL0, surely the target EL should be 3, not 1?

Maybe a default_target_el() function would help (returning
2 if current EL is EL2, 3 if EL3, and 1 or 3 as appropriate
if current EL is EL0 or EL1) ?

Also, what's the plan for handling exceptions which can
trap to EL1, 2 or 3 configurably (eg the various floating
point disables)? Do we put all those trap bits into the TB
flags? If we want to be able to combine them into a single
"FP is disabled" bit in the TB flags, then we need to generate
the same code for any kind of FP-disabled case, which means we
can't encode the target-EL in the generated code like this:
we would need to do something like have a new EXCP_FPDIS
which we then sorted out into an (EXCP_UDEF, target_el, syndrome)
at runtime based on the trap register bits.

thanks
-- PMM