Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: arm32: fix native_intr_handler() #1682

Merged
merged 1 commit into from
Jul 18, 2017

Conversation

jenswi-linaro
Copy link
Contributor

Prior to this patch when a native interrupt was received in IRQ mode
r12 wasn't saved on the stack. There's two problems with that:

  1. r12 isn't a callee preserved register, but must be preserved
    in an exception handler
  2. Without r12 an odd number of registers was saved breaking the
    8 byte alignment of the stack pointer

This patch fixes this by also saving r12 on the stack when handling a
native interrupt in IRQ mode.

Signed-off-by: Jens Wiklander jens.wiklander@linaro.org

@prime-zeng
Copy link
Contributor

prime-zeng commented Jul 13, 2017

@jenswi-linaro it seems this is a important fix, what is the impact if i don't have this patch applied?

@jenswi-linaro
Copy link
Contributor Author

Random crashes in secure world. However, only if OP-TEE is configured for GICv3.

@prime-zeng
Copy link
Contributor

@jenswi-linaro Got it, thank you.

@MrVan
Copy link
Contributor

MrVan commented Jul 13, 2017

R12 is used as an IP register in AAPCS, I think no need to save/restore it. But considering the 8bytes aligned stack requirement, it is ok use r12 or other registers.

Reviewed-by: Peng Fan <peng.fan@nxp.com>

@jenswi-linaro
Copy link
Contributor Author

@MrVan , thanks.
r12/ip isn't preserved by the callee so this register can be corrupted by c-functions used in the interrupt handler. This is bad since the function being interrupted could have been using r12/ip internally between function calls.

@prime-zeng
Copy link
Contributor

prime-zeng commented Jul 13, 2017

Hi jens, from your explanation, the other non-callee preserved registers should also be saved in interrupt handler.

@jenswi-linaro
Copy link
Contributor Author

Yes, all non-callee preserved registers are saved in the interrupt handler with this change.

Prior to this patch when a native interrupt was received in IRQ mode
r12 wasn't saved on the stack. There's two problems with that:
1. r12 isn't a callee preserved register, but must be preserved
   in an exception handler
2. Without r12 an odd number of registers was saved breaking the
   8 byte alignment of the stack pointer

This patch fixes this by also saving r12 on the stack when handling a
native interrupt in IRQ mode.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (Hikey)
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jenswi-linaro jenswi-linaro force-pushed the fix_native_intr_handler branch from ca87f98 to 56f6171 Compare July 18, 2017 14:48
@jenswi-linaro
Copy link
Contributor Author

Rebased, tag applied.

@jforissier jforissier merged commit d824159 into OP-TEE:master Jul 18, 2017
@jenswi-linaro jenswi-linaro deleted the fix_native_intr_handler branch July 18, 2017 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants