Re: [Qemu-arm] [Qemu-devel] [PATCH 02/20] target/arm: Don't switch to target stack early in v7M exception return

[Peter Maydell]

On 5 October 2017 at 17:20, Peter Maydell <address@hidden> wrote:
> On 5 October 2017 at 17:04, Richard Henderson
> <address@hidden> wrote:
>> On 09/22/2017 10:59 AM, Peter Maydell wrote:
>>> +static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool 
>>> threadmode,
>>> +                                bool spsel)
>>> +{
>>> +    /* Return a pointer to the location where we currently store the
>>> +     * stack pointer for the requested security state and thread mode.
>>> +     * This pointer will become invalid if the CPU state is updated
>>> +     * such that the stack pointers are switched around (eg changing
>>> +     * the SPSEL control bit).
>>> +     * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().
>>> +     * Unlike that pseudocode, we require the caller to pass us in the
>>> +     * SPSEL control bit value; this is because we also use this
>>> +     * function in handling of pushing of the callee-saves registers
>>> +     * part of the v8M stack frame, and in that case the SPSEL bit
>>> +     * comes from the exception return magic LR value.
>>
>> Exception return magic lr value does not appear to match "pushing".  Did you
>> mean "poping" here?
>
> No, because the code creates the exception magic LR value for an
> exception entry, and then uses it to determine which SPSEL to use.
> In the tailchained-exception case we use the magic LR that
> the attempted exception-exit got when figuring out where we need
> to push more registers as part of the tailchaining. The pseudocode
> chooses to open-code the "find the right stack pointer" for that
> codepath (in pseudocode function PushCalleeStack), whereas for this
> QEMU code I opted to make the utility function more specific. That's
> what this comment is trying to gesture at.
>
> [The push-on-exception-entry code is in "target/arm: Add v8M support
> to exception entry code"]

I'm going to change this part of the comment to read
     * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().
     * Unlike that pseudocode, we require the caller to pass us in the
     * SPSEL control bit value; this is because we also use this
     * function in handling of pushing of the callee-saves registers
     * part of the v8M stack frame (pseudocode PushCalleeStack()),
     * and in the tailchain codepath the SPSEL bit comes from the exception
     * return magic LR value from the previous exception. The pseudocode
     * opencodes the stack-selection in PushCalleeStack(), but we prefer
     * to make this utility function generic enough to do the job.

which hopefully is a little clearer.

thanks
-- PMM