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

boards: Intel cAVS: disable DTRACE #7955

Merged

Conversation

ujfalusi
Copy link
Contributor

TGL and TGL-H can only be build with IPC4 support and DTRACE is only supported by IPC3.

@ujfalusi ujfalusi requested review from lyakh and kv2019i July 17, 2023 10:04
@ujfalusi ujfalusi force-pushed the peter/pr/intel_cavs_disable_dtrace_01 branch from 3b554e4 to c453ab2 Compare July 17, 2023 10:29
@ujfalusi
Copy link
Contributor Author

SOFCI TEST

@ujfalusi
Copy link
Contributor Author

It is failing all over in check-sof-logger which is really odd since that is using mtrace in case of IPC4 and really, dtrace is not used with IPC4.
Locally the check-sof-logger test just fails on sof:main regardless of CONFIG_TRACE.

ujfalusi added 2 commits July 17, 2023 15:29
DTRACE is IPC3 only, it is not used anymore.

Signed-off-by: Peter Ujfalusi <[email protected]>
DTRACE is IPC3 only, it is not used anymore.

Signed-off-by: Peter Ujfalusi <[email protected]>
@ujfalusi ujfalusi force-pushed the peter/pr/intel_cavs_disable_dtrace_01 branch from c453ab2 to 3829e40 Compare July 17, 2023 12:46
@ujfalusi ujfalusi marked this pull request as draft July 17, 2023 12:47
@ujfalusi
Copy link
Contributor Author

it keeps failing with check-logger, let's see if reverting the TGL config change makes it working (iow, revert to sof:main state).

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Jul 17, 2023

So, on sof: main:

rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py tgl
strings ../build-sof-staging/sof/sof-tgl.ldc | grep -c -i zephyr
25
rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py -i IPC4 tgl
strings ../build-sof-staging/sof/sof-tgl.ldc | grep -c -i zephyr
1
rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py  --fw-naming AVS tgl
strings ../build-sof-staging/sof/tgl/sof-tgl.ldc | grep -c -i zephyr
1

applying the PR (only the first patch is relevant for TGL):

rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py tgl
strings ../build-sof-staging/sof/sof-tgl.ldc | grep -c -i zephyr
1
rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py -i IPC4 tgl
strings ../build-sof-staging/sof/sof-tgl.ldc | grep -c -i zephyr
1
rm -rf ../build-tgl ../build-sof-staging
./scripts/xtensa-build-zephyr.py  --fw-naming AVS tgl
strings ../build-sof-staging/sof/tgl/sof-tgl.ldc | grep -c -i zephyr
1

The check-sof-logger.sh is using the is_zephyr(), which does the above string counting and if the number of zephyr mention is > 10, then it is zephyr, if less, it is XTOS.

Disabling the TRACE somehow reduces the zephyr mentions in the ldc file to 1 (also any command line parameter does the same).
This makes the test fail.
This makes it pass, but it is a hack:

diff --git a/case-lib/lib.sh b/case-lib/lib.sh
index 6318a53ef476..d7b6a3a9f8d2 100644
--- a/case-lib/lib.sh
+++ b/case-lib/lib.sh
@@ -702,7 +702,7 @@ is_zephyr()
     local znum
     znum=$(strings "$ldcFile" | grep -c -i zephyr)
     # As of Nov. 2021, znum ~= 30 for Zephyr and 0 for XTOS
-    test "$znum" -gt 10
+    test "$znum" -gt 0
 }
 
 # FIXME: the kernel driver should give us the FW path

@marc-hb, any hint, idea what is going on?

@ujfalusi
Copy link
Contributor Author

If I download the fw_ipc4/xcc/zephyr/sof-ipc4/tgl/sof-tgl.ldc files form CI storage, the number of zephyr mentions are 1 for all builds, even for the build which should correspond to the one which passes.

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 17, 2023

Hi @ujfalusi , sorry I don't have time to look at this today, hopefully this week. What I can tell you is: sof-test in general (not just check-sof-logger) makes a lot of guesses and this is not good, see thesofproject/linux#3867 for more bad news.

The difference between check-sof-logger and the rest of sof-test is: the other tests tend not to care about totally missing logs and pass anyway! But their brittle detection logic should be the same, they should share the same, relevant sof-test code.

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 17, 2023

Please add a comment telling where CONFIG_TRACE comes from. That name is unfortunately the most vague and generic whereas we have way too many logging solutions still in the main branch (which supports not just Intel)

@ujfalusi
Copy link
Contributor Author

The difference between check-sof-logger and the rest of sof-test is: the other tests tend not to care about totally missing logs and pass anyway! But their brittle detection logic should be the same, they should share the same, relevant sof-test code.

The firmware log collection works on other tests, only check-sof-logger is failing and I think I might have an idea why...
check-sof-logger uses is_zephyr which runs strings counting on ldc file (default: /etc/sof/sof-PLATFORM.ldc)
most other tests use is_firmware_file_zephyr which runs strings on the firmware binary itself.
Based on grep, is_zephyr is only present in:

case-lib/hijack.sh:            if is_zephyr; then
case-lib/lib.sh:is_zephyr()
test-case/check-sof-logger.sh:    if is_zephyr; then
test-case/check-sof-logger.sh:    if is_zephyr; then
test-case/check-sof-logger.sh:        if is_ipc4 && is_zephyr && [ "$f" = 'etrace' ]; then
test-case/check-sof-logger.sh:        elif is_zephyr && [ "$f" = 'etrace' ]; then
test-case/multiple-pipeline.sh:    if [ "$platf" = bdw ] && [ "$f_arg" != 'p' ] && ! is_zephyr; then

is_firmware_file_zephyr works fine with this PR...

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 18, 2023

Thanks @ujfalusi , great catch. I just checked the git log and the .ldc-based is_zephyr is one year older than is_firmware_file_zephyr. Please submit a sof-test switch to the newer function.

Warning: the newer function depends on the firmware having been loaded at least once. So there maybe corner cases where the older function is still required. But the more the newer function can be used the better.

@ujfalusi
Copy link
Contributor Author

@ujfalusi ujfalusi force-pushed the peter/pr/intel_cavs_disable_dtrace_01 branch from 3829e40 to 4e0121f Compare July 19, 2023 13:53
@ujfalusi ujfalusi marked this pull request as ready for review July 19, 2023 13:53
@ujfalusi
Copy link
Contributor Author

sof-test should handle now the logger checking, move out from Draft.

@kv2019i
Copy link
Collaborator

kv2019i commented Jul 20, 2023

Known pause-resume fail on one DUT in https://sof-ci.01.org/sofpr/PR7955/build10953/devicetest/index.html, one PM fail (on one DUT) in the same report. Matches baseline, so proceeding with merge.

@kv2019i kv2019i merged commit 1411dd6 into thesofproject:main Jul 20, 2023
@ujfalusi ujfalusi deleted the peter/pr/intel_cavs_disable_dtrace_01 branch August 17, 2023 06: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