On 5 October 2017 at 17:20, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 5 October 2017 at 17:04, Richard Henderson
> <richard.hender...@linaro.org> 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
