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

Fixes CMSIS v2 test on x86 #30621

Merged
merged 3 commits into from
Dec 17, 2020

Conversation

dcpleung
Copy link
Member

  • In test_event_flags_no_wait_timeout(), after creating thread1, there is a little delay to let it run to trigger FLAG1. However, in test_event_flags_signalled(), this is not being done, and on some platforms it triggers the assert in thread1() complaining the flag not being set. This adds the same delay in test_event_flags_signalled() and the this test passes for those previously failed platforms.
  • Some of the thread tests are hard-coded with assumption that there is only one CPU. So limit the number of CPUs to 1 via kconfig.
  • The conditions which prohibit running the test suite on qemu_x86_64 and up_squared no longer exist. So remove these two from the exclude list, and now they can be built and run in CI.

Fixes #25507

@dcpleung dcpleung requested a review from nashif as a code owner December 11, 2020 02:19
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Dec 11, 2020
@jhedberg
Copy link
Member

@dcpleung this needs to be synced with #30606 since that one is adding another board to the exclude list. Ie whichever PR goes in last will need updating.

@abrodkin abrodkin added the area: SMP Symmetric multiprocessing label Dec 11, 2020
@abrodkin
Copy link
Collaborator

I think @IRISZZW bumped into this as well and was going to suggest the same fix as in 65485c9. So that's good it will be fixed for SMP systems now.

BTW @dcpleung it might make sense that at least 65485c9 is needed for any SMP system.

@nashif
Copy link
Member

nashif commented Dec 11, 2020

@dcpleung this needs to be synced with #30606 since that one is adding another board to the exclude list. Ie whichever PR goes in last will need updating.

this might fix the issue on ehl as well, so no need to exclude.

@nashif
Copy link
Member

nashif commented Dec 11, 2020

@dcpleung this hangs for me on

*** Booting Zephyr OS build zephyr-v2.4.0-2271-g89c456d897fc  ***
Running test suite test_cmsis_v2_apis
===================================================================
START - test_kernel_apis
 PASS - test_kernel_apis
===================================================================
START - test_delay

@jhedberg
Copy link
Member

this might fix the issue on ehl as well, so no need to exclude.

I'm almost sure it does fix it, which was my point: if my PR gets merged first then this PR needs rebasing, and if this PR gets merged first then I need to update my PR to not try to add anything to the excludes.

@dcpleung
Copy link
Member Author

@dcpleung this hangs for me on

*** Booting Zephyr OS build zephyr-v2.4.0-2271-g89c456d897fc  ***
Running test suite test_cmsis_v2_apis
===================================================================
START - test_kernel_apis
 PASS - test_kernel_apis
===================================================================
START - test_delay

Hm... ran a couple times on my boards back-to-back without any issues. Let me try run it more times and see if I can reproduce this.

@dcpleung
Copy link
Member Author

dcpleung commented Dec 11, 2020

Ran 15 times back-to-back without hanging but I had CONFIG_PCIE_MMIO_CFG=n.

@dcpleung
Copy link
Member Author

Found the issue with hanging under test_delay. It is caused by the same issue as in #30574 on CONFIG_INTEL_VTD_ICTL being enabled.

@dcpleung dcpleung requested a review from chen-png December 11, 2020 22:19
@jhedberg
Copy link
Member

jhedberg commented Dec 12, 2020

@dcpleung #30606 is now merged so you'll need to rebase and fix the conflict in testcase.yaml. You can just remove the ehl_crb boad like you're doing with up_squared.

In test_event_flags_no_wait_timeout(), after creating thread1,
there is a little delay to let it run to trigger FLAG1.
However, in test_event_flags_signalled(), this is not being
done, and on some platforms it triggers the assert in thread1()
complaining the flag not being set. This adds the same delay
in test_event_flags_signalled() and the this test passes for
those previously failed platforms.

Signed-off-by: Daniel Leung <[email protected]>
Some of the thread tests are hard-coded with assumption that
there is only one CPU. So limit the number of CPUs to 1
via kconfig.

Signed-off-by: Daniel Leung <[email protected]>
The conditions which prohibit running the test suite on
qemu_x86_64, up_squared and ehl_crb no longer exist. So
remove these two from the exclude list, and now they can
be built and run in CI.

Signed-off-by: Daniel Leung <[email protected]>
@dcpleung
Copy link
Member Author

@dcpleung #30606 is now merged so you'll need to rebase and fix the conflict in testcase.yaml. You can just remove the ehl_crb boad like you're doing with up_squared.

Thanks for letting me know. Rebased and removed all x86 64-bit boards from the exclude list.

Copy link
Collaborator

@chen-png chen-png left a comment

Choose a reason for hiding this comment

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

tested and it fixed 25507

@jhedberg jhedberg merged commit 856fd65 into zephyrproject-rtos:master Dec 17, 2020
@dcpleung dcpleung deleted the x86_cmsis_v2_fixes branch December 17, 2020 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: SMP Symmetric multiprocessing area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

up_squared:tests/portability/cmsis_rtos_v2 failed.
5 participants