Skip to content

Add FPU Context Handling for ARM_AARCH64_SRE port #1229

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

Conversation

asellaminxp
Copy link

Add FPU Context Handling for ARM_AARCH64_SRE port.

Description

  • Add support for vApplicationFPUSafeIRQHandler for ARM_AARCH64_SRE port, similar to the work done in #1113.
  • Add the configuration and status registers to the fpu saved context for ARM_AARCH64_SRE port.
  • Add configUSE_TASK_FPU_SUPPORT support for ARM_AARCH64_SRE port. Ported from #1048.

Test Steps

Tested on our example applications NXP/harpoon-apps, which are demo applications running on Cortex-A using the jailhouse hypervisor with Linux running and FreeRTOS as a guest OS.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This is a direct backport of upstream commit [1] for aarch64 (legacy operation port)
done under [2]
The same code can be applied on the aarch SRE port to be able to enable FPU context
saving on all tasks context switch to mitigate GCC optimization to use SIMD registers
for copy.

[1] "55eceb22: Add configUSE_TASK_FPU_SUPPORT to AARCH64 port (FreeRTOS#1048)"
[2] FreeRTOS#1048

Signed-off-by: Marouen Ghodhbane <marouen.ghodhbane@nxp.com>
…the fpu saved context

FPSR and FPCR are two 64-bits registers where only the lower 32 bits are defined.
Save them when doing context switch with FPU context saving enabled.

Signed-off-by: Marouen Ghodhbane <marouen.ghodhbane@nxp.com>
The application writer needs to name their IRQ handler as:
1. vApplicationIRQHandler if the IRQ handler does not use FPU registers.
2. vApplicationFPUSafeIRQHandler is the IRQ handler uses FPU registers.

When the application uses vApplicationFPUSafeIRQHandler, a default
implementation of vApplicationIRQHandler is used which stores FPU
registers and then calls vApplicationFPUSafeIRQHandler.

Note that recent versions of GCC may use FP/SIMD registers to optimize 16-bytes
copy and especially when using va_start()/va_arg() functions (e.g printing some thing
in IRQ handlers may trigger usage of FPU registers)

This implementation is heavily inspired by both the ARM_CA9 port and the ARM_CRx_No_GIC
port done in [1]

[1] FreeRTOS#1113

Signed-off-by: Marouen Ghodhbane <marouen.ghodhbane@nxp.com>
@asellaminxp asellaminxp requested a review from a team as a code owner January 22, 2025 17:12
Copy link

@archigup archigup merged commit 72bb476 into FreeRTOS:main Jan 23, 2025
17 checks passed
@asellaminxp asellaminxp deleted the upstream/aarch64_sre_add_fpu_context_save_support branch January 23, 2025 08:11
@aggarg
Copy link
Member

aggarg commented Jan 23, 2025

@asellaminxp I am trying to understand why we need to save FPCR and FPSR as part of the context -

  • Floating-point Control Register (FPCR) - Controls floating-point extension behavior. I do not think floating point extension behavior will change from task to task.
  • Floating-point Status Register (FPSR) - Provides floating-point system status information. But this registers only saves AArch32 floating point comparison flags. AArch64 floating-point comparisons set flags in the PSTATE register instead.

The non SRE port does not save FPCR and FPSR as part of the context either. Would you please help me understand why we need to save these registers as part of the context.

@GhMarwen
Copy link
Contributor

Hi @aggarg

The save/restore for the status and control registers was more of a precaution (and by comparing to what other implementations are doing) than a targeted fix (As a general rule of thumb, we tend to save the control and status registers as part of the context).
I was more concerned about the condition flags but you're right that's only for the AARCH32 state and I was/still in doubt about the FPCR changes in between tasks (hence the try to check other FPU save implementations out there). Also, since the memory footprint and the performance penalty did not seem to be considerable for an AARCH64, I went for the conservative way.
Please let us know if you think that's an overkill/useless and we should revert it anyway ? :-)

CC @asellaminxp

@aggarg
Copy link
Member

aggarg commented Jan 24, 2025

Thank you for your response @GhMarwen! At the moment, it is inconsistent between SRE and non-SRE ports for AArch64. I'd prefer to keep it same in both the ports. I have reached out to Arm to ask about their suggestion and based on what I hear from them, we'll make both the ports same. Will keep you posted.

@AhmedIsmail02
Copy link
Member

Hi @aggarg and @GhMarwen,

FPSR and FPCR are both state controllable by the running process (EL0 code). Hence, they need to be saved and restored by context switches in exactly the same way all the general purpose registers are. This applies to both ARM_AARCH64 and ARM_ARRCH64_SRE ports, since the changes were already done to ARM_AARCH64_SRE port as part of this PR, then we just need to apply the same changes for the ARM_ARRCH64 port as well.

Thanks

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.

6 participants