Skip to content

Tests arch arm swap #17423

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 14, 2019
Merged

Tests arch arm swap #17423

merged 4 commits into from
Jul 14, 2019

Conversation

ioannisg
Copy link
Member

@ioannisg ioannisg commented Jul 9, 2019 •
edited
Loading

A coverage test for the ARM thread swap mechanism.

Covers:

  • thread state update and restore (mode, base-priority, swap-return value)
  • callee-saved registers' preservation and restore
  • extension for floating-point support

Currently rebased on top of #17456

@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Jul 9, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jul 9, 2019 •
edited
Loading

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from fd10b5c to 60d26b0 Compare July 10, 2019 10:06
@ioannisg ioannisg requested a review from pizi-nordic July 10, 2019 11:58
@ioannisg
Copy link
Member Author

@pizi-nordic what do you think about this functional test for the arm swap routine? Any early feedback is appreciated

@lemrey
Copy link
Collaborator

lemrey commented Jul 10, 2019

Have a look at #17456, it should fix the compiler warning.

pizi-nordic
pizi-nordic reviewed Jul 10, 2019
View reviewed changes
@pizi-nordic
Copy link
Collaborator

IMHO what is missing is a high level description of the idea behind the test.

@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from 60d26b0 to 7eafdd7 Compare July 10, 2019 15:24
@ioannisg
Copy link
Member Author

IMHO what is missing is a high level description of the idea behind the test.

Do you think I sould add a README? Or an intro paragraph in the test source file?
We do have such things in some tests, but it is not a formal requirement

@ioannisg ioannisg added the area: ARM ARM (32-bit) Architecture label Jul 10, 2019
@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from 7eafdd7 to a62390e Compare July 10, 2019 21:18
@ioannisg ioannisg marked this pull request as ready for review July 10, 2019 21:54
@ioannisg ioannisg added the Enhancement Changes/Updates/Additions to existing features label Jul 10, 2019
@pizi-nordic
Copy link
Collaborator

IMHO what is missing is a high level description of the idea behind the test.

Do you think I sould add a README? Or an intro paragraph in the test source file?
We do have such things in some tests, but it is not a formal requirement

Just comment in the file. It might be also in emphasized comment in function like this:

/*
 * STEP 3: Verify if all registers were preserved
 */

/* Verify callee-saved registers */
/* Verify FPU registers */
/* Verify BASEPRI */
..

@ioannisg
Copy link
Member Author

IMHO what is missing is a high level description of the idea behind the test.

Do you think I sould add a README? Or an intro paragraph in the test source file?
We do have such things in some tests, but it is not a formal requirement

Just comment in the file. It might be also in emphasized comment in function like this:

/*
 * STEP 3: Verify if all registers were preserved
 */

/* Verify callee-saved registers */
/* Verify FPU registers */
/* Verify BASEPRI */
..

I already added a README description, @pizi-nordic , please, take a look.

@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from a62390e to fc0d00b Compare July 11, 2019 10:10
@ioannisg
Copy link
Member Author

@pizi-nordic @SebastianBoe could you do another (hopefully) quick review round?

@ioannisg ioannisg mentioned this pull request Jul 11, 2019
Closed
26 tasks
pizi-nordic
pizi-nordic approved these changes Jul 11, 2019
View reviewed changes
Copy link
Collaborator

@pizi-nordic pizi-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Place consider adding irq_lock() around all code except places where you should be pre-empted.

@ioannisg
Copy link
Member Author

Place consider adding irq_lock() around all code except places where you should be pre-empted.

Done, as well, thanks @pizi-nordic for reviewing.

@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from fc0d00b to 19ea3db Compare July 11, 2019 11:09
@ioannisg ioannisg removed the request for review from lemrey July 12, 2019 08:03
ioannisg added 4 commits July 12, 2019 11:08
@ioannisg
tests: arch: arm: contribute a test for arm swap
044679d 
This commit contributes a test for the ARM swap, i.e. the
context-switch mechanism for the ARM architecture. The test
verifies that the thread state variables are set and checked
properly when performing a thread swap-out and swap-in.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
tests: arch: arm: enhance swap test for FP sharing
6e0bec5 
We add a test-case for arch/arm/thread_swap, so the
test is executed with CONFIG_FLOAT/CONFIG_FP_SHARING
being enabled, if an FPU is available. We execute the
test extension with/without enabling compiler optimizations.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
tests: arch: arm_thread_swap: add README file
d3349cf 
The commit adds a README file with a description of the test.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg
CODEOWNERS: adding code-owner for arch/arm test suite
655e7f8 
Assigning a code-owner for all tests under tests/arch/arm.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
@ioannisg ioannisg force-pushed the tests_arch_arm_swap branch from 19ea3db to 655e7f8 Compare July 12, 2019 09:18
@nashif nashif merged commit 138c38b into zephyrproject-rtos:master Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nashif nashif nashif approved these changes

@SebastianBoe SebastianBoe SebastianBoe approved these changes

@andrewboie andrewboie andrewboie approved these changes

@pizi-nordic pizi-nordic pizi-nordic approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

Assignees
No one assigned
Labels
area: ARM ARM (32-bit) Architecture area: Boards area: Code Coverage area: Tests Issues related to a particular existing or missing test Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ioannisg @zephyrbot @lemrey @pizi-nordic @nashif @SebastianBoe @andrewboie