ASoC: SOF: debug: show firmware/topology prefix/names #4902

plbossart · 2024-04-03T19:19:40Z

We currently log the firmware and topology names in the kernel logs, but there's been an ask to add the information in debugfs to simplify CI scripts.

This isn't meant to be a stable ABI used by apps, changes will be allowed as needed.

Closes: #3867

First step to remove use of kernel parameters Link: thesofproject/linux#4902 Link: thesofproject#1166 Signed-off-by: Pierre-Louis Bossart <[email protected]>

marc-hb

Lovely

sound/soc/sof/debug.c

marc-hb

an ask to add the information in debugfs to simplify CI scripts.

The TEST scripts, NOT the CI scripts.

= the scripts that EVERYONE should run, not the obscure Jenkins scripts or Github workflows no one knows about.

I mean this PR is not to please one particular and demanding user. It's useful for everyone interested in testing their code (= too few people?)

sound/soc/sof/debug.c

First step to remove use of kernel parameters Link: thesofproject/linux#4902 Link: thesofproject#1166 Signed-off-by: Pierre-Louis Bossart <[email protected]>

plbossart · 2024-04-04T21:31:45Z

@ujfalusi This PR was at least useful to detect that we never had a default_lib_path for LNL, see second commit...

ujfalusi · 2024-04-05T14:04:38Z

@plbossart, one checkpatch issue should be addressed.

The SOF driver has multiple profiles to select firmware/topology prefix/names depending on the platform and ipc_type, and each of those fields can be overridden with kernel parameters. This results in some cases in confusion on what configuration is actually used in a given test. We currently log the firmware and topology names in the kernel logs, but there's been an ask to add the information in debugfs to simplify test scripts used by developers and CI. This isn't meant to be a stable ABI used by apps, changes will be allowed as needed. Closes: thesofproject#3867 Signed-off-by: Pierre-Louis Bossart <[email protected]>

The commit cd6f2a2 ("ASoC: SOF: Intel: Set the default firmware library path for IPC4") added the default_lib_path field for all platforms, but this was missed when LunarLake was later introduced. Fixes: 64a63d9 ("ASoC: SOF: Intel: LNL: Add support for Lunarlake platform") Signed-off-by: Pierre-Louis Bossart <[email protected]>

plbossart · 2024-04-05T14:18:54Z

@plbossart, one checkpatch issue should be addressed.

Done. I didn't realize my post-commit hook was missing.

First step to remove use of kernel parameters Link: thesofproject/linux#4902 Link: thesofproject#1166 Signed-off-by: Pierre-Louis Bossart <[email protected]>

First step to remove use of kernel parameters Link: thesofproject/linux#4902 Link: #1166 Signed-off-by: Pierre-Louis Bossart <[email protected]>

marc-hb · 2024-04-22T23:40:11Z

sound/soc/sof/debug.c

+	debugfs_create_str("fw_path", 0444, fw_profile,
+			   (char **)&plat_data->fw_filename_prefix);
+	debugfs_create_str("fw_lib_path", 0444, fw_profile,
+			   (char **)&plat_data->fw_lib_prefix);


There is page_fault_oops with fw_lib_path in some configurations
Found by... testing!

lib.sh: need sudo bash -c ... to expand fw_profile/* as root sof-test#1178

root@jf-cml-rvp-sdw-3:~# cat /sys/kernel/debug/sof/fw_profile/fw_lib_path Killed Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: BUG: kernel NULL pointer dereference, address: 0000000000000000 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: #PF: supervisor read access in kernel mode Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: #PF: error_code(0x0000) - not-present page Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: PGD 0 P4D 0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: Oops: 0000 [#5] PREEMPT SMP NOPTI Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: CPU: 8 PID: 2096 Comm: grep Tainted: G D 6.9.0-rc3-g0c12720aaa21 #dev Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: Hardware name: Intel Corporation CometLake Client Platform/CometLake U DDR4 HR RVP, BIOS CMLSFWR1.R00.1245.D00.1906> Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: RIP: 0010:strlen+0x4/0x30 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: Code: f7 75 ec 31 c0 c3 cc cc cc cc 48 89 f8 c3 cc cc cc cc 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 9> Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: RSP: 0018:ffffaa4b438c7e28 EFLAGS: 00010246 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: RAX: ffff8d2750c389b8 RBX: ffff8d2743c6b800 RCX: 0000000000000006 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: RDX: 0000000000000007 RSI: 0000558abe473000 RDI: 0000000000000000 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: RBP: ffff8d2743c6b800 R08: 0000000000000000 R09: 0000000000000000 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: R13: 0000000000018000 R14: ffffaa4b438c7f08 R15: ffff8d275f0ce8f8 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: FS: 00007f942669e740(0000) GS:ffff8d2893600000(0000) knlGS:0000000000000000 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: CR2: 0000000000000000 CR3: 000000013bdf6004 CR4: 00000000003706f0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: Call Trace: Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: <TASK> Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? __die+0x24/0x70 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? page_fault_oops+0x15a/0x450 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? exc_page_fault+0x68/0x1e0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? asm_exc_page_fault+0x26/0x30 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? strlen+0x4/0x30 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: debugfs_read_file_str+0x4d/0xf0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: vfs_read+0xab/0x340 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ? __do_sys_newfstatat+0x35/0x60 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: ksys_read+0x69/0xf0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: do_syscall_64+0xad/0x1b0 Apr 22 23:33:23 jf-cml-rvp-sdw-3 kernel: entry_SYSCALL_64_after_hwframe+0x72/0x7a

this is a CML platform....

I can't recall is fw_lib_path is relevant for IPC3 @ujfalusi, we could just skip the file create for IPC3 I guess.

At any rate the scripts only used this for IPC4 platforms.

Yes I confirm the oops happened only with the stable-v2.2 firmware branch. But it did happen with ALL 3 devices tested for that branch.

Because of various sudo issues (hopefully last fix in thesofproject/sof-test#1178) fw_lib_path had never been tested yet.

Yes, fw_lib_path is going to be NULL with IPC3, probably we can have have a static const "N/A" string here and give that to debugfs_create_str()?
Or skip creating the file in case of IPC3, it is not applicable after all.

@marc-hb what's the preferred result for scripts? A N/A reply or the file not created at all?

Either can be scripted easily but no file at all is safer because there's less risk of confusing it with something valid:

https://en.wikipedia.org/wiki/Null_pointer a.k.a. the "billion dollar mistake".

Note we have no script trying to read this brand new field yet (= how I just found this bug)

plbossart requested a review from ranj063 as a code owner April 3, 2024 19:19

plbossart requested review from marc-hb, bardliao and ujfalusi April 3, 2024 19:20

plbossart mentioned this pull request Apr 3, 2024

[BUG] sof-test scripts shall not assume settings based on kernel parameters thesofproject/sof-test#1166

Closed

plbossart mentioned this pull request Apr 3, 2024

case-lib: is_ipc4: use new debugfs file instead of kernel parameter thesofproject/sof-test#1167

Merged

marc-hb previously approved these changes Apr 3, 2024

View reviewed changes

ujfalusi reviewed Apr 4, 2024

View reviewed changes

sound/soc/sof/debug.c Outdated Show resolved Hide resolved

bardliao previously approved these changes Apr 4, 2024

View reviewed changes

plbossart dismissed stale reviews from bardliao and marc-hb via 58c830e April 4, 2024 16:14

plbossart force-pushed the fix/debugfs-additions branch from 29b4652 to 58c830e Compare April 4, 2024 16:14

plbossart requested review from ujfalusi, marc-hb and bardliao April 4, 2024 16:15

marc-hb previously approved these changes Apr 4, 2024

View reviewed changes

sound/soc/sof/debug.c Outdated Show resolved Hide resolved

plbossart dismissed marc-hb’s stale review via c82788b April 4, 2024 17:15

plbossart force-pushed the fix/debugfs-additions branch from 58c830e to c82788b Compare April 4, 2024 17:15

plbossart force-pushed the fix/debugfs-additions branch from c82788b to 39a5f1d Compare April 4, 2024 20:06

plbossart requested review from lgirdwood, kv2019i and dbaluta as code owners April 4, 2024 21:19

plbossart requested a review from marc-hb April 4, 2024 21:20

marc-hb previously approved these changes Apr 4, 2024

View reviewed changes

ranj063 previously approved these changes Apr 4, 2024

View reviewed changes

ujfalusi previously approved these changes Apr 5, 2024

View reviewed changes

plbossart added 2 commits April 5, 2024 09:18

plbossart dismissed stale reviews from ujfalusi, ranj063, and marc-hb via 67e1b4f April 5, 2024 14:18

plbossart force-pushed the fix/debugfs-additions branch from 5dd8b15 to 67e1b4f Compare April 5, 2024 14:18

plbossart requested review from ranj063, ujfalusi and marc-hb April 5, 2024 14:19

ujfalusi approved these changes Apr 5, 2024

View reviewed changes

bardliao approved these changes Apr 8, 2024

View reviewed changes

ujfalusi merged commit da76d0e into thesofproject:topic/sof-dev Apr 8, 2024
10 of 14 checks passed

marc-hb reviewed Apr 22, 2024

View reviewed changes

marc-hb mentioned this pull request Apr 22, 2024

lib.sh: need sudo bash -c ... to expand fw_profile/* as root thesofproject/sof-test#1178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASoC: SOF: debug: show firmware/topology prefix/names #4902

ASoC: SOF: debug: show firmware/topology prefix/names #4902

plbossart commented Apr 3, 2024

marc-hb left a comment

marc-hb left a comment •

edited

Loading

plbossart commented Apr 4, 2024

ujfalusi commented Apr 5, 2024

plbossart commented Apr 5, 2024

marc-hb Apr 22, 2024

plbossart Apr 22, 2024 •

edited

Loading

marc-hb Apr 23, 2024 •

edited

Loading

ujfalusi Apr 23, 2024 •

edited

Loading

plbossart Apr 23, 2024

marc-hb Apr 23, 2024

ASoC: SOF: debug: show firmware/topology prefix/names #4902

ASoC: SOF: debug: show firmware/topology prefix/names #4902

Conversation

plbossart commented Apr 3, 2024

marc-hb left a comment

Choose a reason for hiding this comment

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

plbossart commented Apr 4, 2024

ujfalusi commented Apr 5, 2024

plbossart commented Apr 5, 2024

marc-hb Apr 22, 2024

Choose a reason for hiding this comment

plbossart Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

marc-hb Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

ujfalusi Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

plbossart Apr 23, 2024

Choose a reason for hiding this comment

marc-hb Apr 23, 2024

Choose a reason for hiding this comment

marc-hb left a comment •

edited

Loading

plbossart Apr 22, 2024 •

edited

Loading

marc-hb Apr 23, 2024 •

edited

Loading

ujfalusi Apr 23, 2024 •

edited

Loading