Re: [Qemu-devel] [PATCH v8 01/27] target-arm: extend async excp masking

[Peter Maydell]

On 30 October 2014 21:28, Greg Bellows <address@hidden> wrote:
> This patch extends arm_excp_unmasked() to use lookup tables for determining
> whether IRQ and FIQ exceptions are masked.  The lookup tables are based on the
> ARMv8 and ARMv7 specification physical interrupt masking tables.
>
> If EL3 is using AArch64 IRQ/FIQ masking is ignored in all exception levels
> other than EL3 if SCR.{FIQ|IRQ} is set to 1 (routed to EL3).
>
> Signed-off-by: Greg Bellows <address@hidden>
>
> ---
>
> v7 -> v8
> - Add IRQ and FIQ exeception masking lookup tables.
> - Rewrite patch to use lookup tables for determining whether an excpetion is
>   masked or not.
>
> v5 -> v6
> - Globally change Aarch# to AArch#
> - Fixed comment termination
>
> v4 -> v5
> - Merge with v4 patch 10
> ---
>  target-arm/cpu.h | 218 
> ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 190 insertions(+), 28 deletions(-)

Having got through the rest of the series I'm coming back
to this one, which I skipped because it needed checking
against the ARM ARM.

I definitely don't like the use of tables here -- it is in almost
all of the cases simply repeating the routing calculations.
All you actually need here is:

    if (target_el < current_el) {
        /* Don't take exception */
        return false;
    }
    if (target_el > current_el && target_el != 1) {
        if (target_el == 3 && !arm_el_is_aa64(env, 3)) {
            /* In a 32 bit EL3 there are some awkward special cases where we
             * must honour the PSTATE/CPSR mask bits even when taking the
             * exception to EL3.
             */
            if (arm_is_secure(env)) {
                goto honour_masking;
            }
            /* (We know at this point that SCR.FIQ/IRQ must be set.) */
            if (excp_idx == EXCP_FIQ && HCR.FMO == 0 && SCR.FW == 1) {
                goto honour_masking;
            }
            /* If we supported SError then the async external abort routing
             * would have a similar case for SCR.AW here. There is no
SCR.IW for IRQs.
             */
            if (excp_idx == EXCP_IRQ && HCR.IMO == 0) {
                goto honour_masking;
            }
       }
       /* Take the exception unconditionally. */
       return true;
    }
honour_masking:
    /* If we get here then we check the PSTATE flags. */

(I think the 'gotos' here are clearer than turning the if statement inside out
to avoid them...)

> -    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> -    /* FIXME: Use actual secure state.  */
> -    bool secure = false;
> -    /* If in EL1/0, Physical IRQ routing to EL2 only happens from NS state.  
> */
> -    bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> -    /* ARMv7-M interrupt return works by loading a magic value
> -     * into the PC.  On real hardware the load causes the
> -     * return to occur.  The qemu implementation performs the
> -     * jump normally, then does the exception return when the
> -     * CPU tries to execute code at the magic address.
> -     * This will cause the magic PC value to be pushed to
> -     * the stack if an interrupt occurred at the wrong time.
> -     * We avoid this by disabling interrupts when
> -     * pc contains a magic address.

You'll probably find it easier to base on top of the patches
I sent out that split out the M profile code from this
function (I'm planning to put those into target-arm.next soon
so they get in before hardfreeze).

thanks
-- PMM