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

Work around SysTick bug for QEMU ARMv8-M #724

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

jefftenney
Copy link
Contributor

@jefftenney jefftenney commented Jul 27, 2023

Work around SysTick bug for QEMU ARMv8-M

Description

QEMU versions older than 7.0.0 contain a bug which causes an emulation error and application failure if port code enables SysTick without first selecting a valid clock source. Prior to this PR, the port code triggered the bug by attempting to change clock sources from a clock with a zero clock period (the "alternate" clock) to one with a nonzero clock period (the core clock) and enable SysTick at the same time. After this PR, the port configures the CLKSOURCE bit first and then sets the ENABLE bit. This workaround avoids the bug in QEMU versions older than 7.0.0.

QEMU versions 7.0.0 and newer are not (easily) available to the current LTS version of Ubuntu (22.04). So a workaround is appropriate.

It seems this QEMU bug does not affect ARMv6-M and ARMv7-M targets. Apparently that is because QEMU forces the CLKSOURCE bit to 1 for any write by the port code for those targets. The port code attempts to write a zero before enabling the SysTick so QEMU is setting the CLKSOURCE bit early for us on those ports.

Note that the QEMU bug cannot be triggered by the tickless-idle code, even though that code enables/disables SysTick freely and even changes the clock source temporarily when the user's FreeRTOS configuration selects the "alternate" clock. The tickless-idle code avoids the bug for both clock configurations:

  • SysTick clocked by core clock: The tickless logic never changes the CLKSOURCE bit.
  • SysTick clock by alternate clock: The tickless logic never selects an invalid clock source. If the alternate clock source is invalid, then the user's FreeRTOS configuration is invalid. QEMU will display an error message during execution of vPortSetupTimerInterrupt() indicating the selected clock has a zero clock period, which is the right message for the user's invalid FreeRTOS config. It's important to note that the bug is not triggered by merely changing the CLKSOURCE bit while simultaneously enabling SysTick. Instead, the bug is triggered by changing from an invalid clock to a valid clock while simultaneously enabling SysTick.

Test Steps

  • Start with a working QEMU setup on Ubuntu 22.04 with QEMU 6.2.0.
  • Checkout paulbartell/add-generic-arm-mps-project. This project is a work in progress but it functions well enough to test this workaround.
  • Work from FreeRTOS/FreeRTOS/Demo/ARM_MPS/.
  • Use CMake to create the build config
  • Make the mps2_an505_cm33_blink_demo.elf target from FreeRTOS/FreeRTOS/Demo/ARM_MPS/.
  • Run "qemu-system-arm -machine mps2-an505 -m 16M -cpu cortex-m33 -nographic -semihosting -monitor null --semihosting-config enable=on,target=native -kernel mps2_an505_cm33_blink_demo.elf -d guest_errors"
jeff@ubuntu-vm:~/FreeRTOS/FreeRTOS/Demo/ARM_MPS$ qemu-system-arm -machine mps2-an505 -m 16M -cpu cortex-m33 -nographic -semihosting -monitor null --semihosting-config enable=on,target=native -kernel mps2_an505_cm33_blink_demo.elf -d guest_errors
Invalid read at addr 0x10000000, size 4, region '(null)', reason: rejected
Invalid read at addr 0x10000004, size 4, region '(null)', reason: rejected
Timer with period zero, disabling
  • Apply this PR (ok to just get portable/ARMv8M/non_secure/port.c as that file is used directly by the project)
  • Make again
  • Run again
jeff@ubuntu-vm:~/FreeRTOS/FreeRTOS/Demo/ARM_MPS$ qemu-system-arm -machine mps2-an505 -m 16M -cpu cortex-m33 -nographic -semihosting -monitor null --semihosting-config enable=on,target=native -kernel mps2_an505_cm33_blink_demo.elf -d guest_errors
Invalid read at addr 0x10000000, size 4, region '(null)', reason: rejected
Invalid read at addr 0x10000004, size 4, region '(null)', reason: rejected
blinking
blinking
blinking
  • Edit ARM_MPS/Include/FreeRTOSConfig.h to add "#define configUSE_TICKLESS_IDLE 1"
  • Make again
  • Run again (note same successful output as before)

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. (N/A)

Related Issue

If merged, should probably close #660.

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

The workaround now uses portNVIC_SYSTICK_CLK_BIT_CONFIG instead of
portNVIC_SYSTICK_CLK_BIT, which saves us from having to explain in the
comments why it's OK to temporarily set the CLKSOURCE bit even if the
user's FreeRTOS configuration clears the CLKSOURCE bit.

Using portNVIC_SYSTICK_CLK_BIT_CONFIG here still correctly prevents the
firmware from triggering the QEMU bug.
@jefftenney jefftenney requested a review from a team as a code owner July 27, 2023 01:33
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d02ab77) 94.35% compared to head (6a9d5d1) 94.35%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #724   +/-   ##
=======================================
  Coverage   94.35%   94.35%           
=======================================
  Files           6        6           
  Lines        2443     2443           
  Branches      598      598           
=======================================
  Hits         2305     2305           
  Misses         85       85           
  Partials       53       53           
Flag Coverage Δ
unittests 94.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aggarg
Copy link
Member

aggarg commented Jul 27, 2023

Thank you for your contribution @jefftenney!

@aggarg aggarg merged commit b13e269 into FreeRTOS:main Jul 27, 2023
@jefftenney jefftenney deleted the accommodate-old-QEMU-SysTick-bug branch July 27, 2023 06:00
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.

3 participants