ASoC: SOF: ext_manifest: Initial submission by ktrzcinx · Pull Request #1903 · thesofproject/linux

ktrzcinx · 2020-03-18T07:07:43Z

Some minor fixes proposed by @lyakh in #1818

kv2019i · 2020-03-18T10:30:10Z

@ktrzcinx We need to squash these changes to the merged series, so can you update this to edit every commit to have summary of:

"squash! ASoC: SOF: ext_manifest: parse compiler version"

.. with the summary matching the merged commit this patch should be squashed into (before sending upstream). This syntax is used by "git commit --squash " and you can also use it with amending "git commit --amend --squash .

plbossart · 2020-03-18T18:14:39Z

Agree with @kv2019i without clear indications it's likely maintainers will make a mistake when squashing. please provide guidance on what needs to be squashed where, thank you

ranj063 · 2020-03-19T16:38:59Z

@kv2019i how does the !fixup work? Will be autosquash the patches when we click on rebase and merge the PR?

kv2019i · 2020-03-20T08:15:35Z

@ranj063 wrote:

@kv2019i how does the !fixup work? Will be autosquash the patches when we click on rebase and merge the PR?

No, it's not done by GitHub. But when we do a rebase of sof-dev-rebase, then the fixup/squash commits are automatically ordered such that we can easily combine them. You need to have "autoSquash = true" in your .git/config, or pass "--autosquash" to git-rebase.

We never rebase sof-dev, so in sof-dev these patches will always stay as they are.

This may cause some complications to our scripts for picking reviewed-by tags, though, so something to watch out for. FYI @plbossart

kv2019i

Thanks @ktrzcinx . One comment on the include path, and I'd also like to double-confirm what is the final FW version of the header, so we don't need further fixups. I.e. let the FW patch be merged first.

kv2019i · 2020-03-20T08:17:06Z

-	uint8_t blob[]; /*< type dependent content */
+
+	/* just after this header should be type dependent content */
 } __packed;


To avoid further fixups, I want confirmation that the FW side is merged.

@ktrzcinx Why are you removing this field? Without it getting to the data would become more difficult. Now you can just do hdr->blob, after this patch this won't be any longer possible. Above you did keep a variable-length .elements[] array, why are you removing .blob[] here?

@lyakh thesofproject/sof#2427 (comment). Moreover hdr->blob shouldn't be used at all, hdr should be every time casted to type specific struct. Field .elements[] should be deleted in a same way as .blob[], my mistake.

kv2019i

Thanks @ktrzcinx -- this has been one long PR. Let's wait for the other reviewer's to ack and acceptance for sof#2427

kv2019i · 2020-03-24T07:55:30Z

All good, now just waiting for thesofproject/sof#2427 to be approved.

UPDATE: still waiting for thesofproject/sof#2427 to be merged (some extra steps needed to keep QEMU in CI going on)

Extended manifest is a place to store build time known firmware metadata, for example firmware version or used compiler description. Given information is read on host side before firmware startup. This part of output binary is located as a first structure in binary file. Extended manifest should be skipped in firmware loading routine. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

The firmware version can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

kv2019i

Thanks @ktrzcinx , looks good now! I actually made some comments yesterday, but those had not been sent. But you addressed those in the last update, so looks good!

lyakh · 2020-05-04T06:15:25Z

+	size_t w_size = struct_size(w, window, w->num_windows);
+
+	if (sdev->info_window) {
+ if (memcmp(sdev->info_window, w, w_size)) {


you compare w_size of memory here, and w_size is calculated from w->num_windows, and w->num_windows is only verified below to be valid. So, for example if w->num_windows == 0 the below check would return -EINVAL, but in case of sdev->info_window != NULL you now return 0 in this case? Which one is correct?

Such an issue would be properly handled, because for w->num_windows == 0w->num_windows will differ from sdev->info_window->num_windows for sure (condition in line 24/33). Event that I changed order of those checks, to make it more obvious.

The window description can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

The compiler version and description can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

kv2019i · 2020-05-05T12:09:03Z

@ranj063 This is ready to go, but you still have pending changes-requested, so can you check that?

marc-hb · 2020-05-09T20:57:51Z

Please review the 2-lines long, PR #2091 fixup! for the crash below

May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: use msi interrupt mode
May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: hda codecs found, mask 4
May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: using HDA machine driver skl_hda_dsp_generic now
May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: DMICs detected in NHLT tables: 0
May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: Direct firmware load for intel/sof/community/sof-apl.ri failed with error -2
May 08 17:20:22 up2 kernel: sof-audio-pci 0000:00:0e.0: error: request firmware intel/sof/community/sof-apl.ri failed err: -2
May 08 17:20:22 up2 kernel: BUG: kernel NULL pointer dereference, address: 0000000000000000
May 08 17:20:22 up2 kernel: #PF: supervisor read access in kernel mode
May 08 17:20:22 up2 kernel: #PF: error_code(0x0000) - not-present page
May 08 17:20:22 up2 kernel: PGD 0 P4D 0
May 08 17:20:22 up2 kernel: Oops: 0000 [#1] SMP NOPTI
May 08 17:20:22 up2 kernel: CPU: 3 PID: 86 Comm: kworker/3:1 Not tainted 5.7.0-rc3-sof+ #12
May 08 17:20:22 up2 kernel: Hardware name: AAEON UP-APL01/UP-APL01, BIOS UPA1AM40 08/06/2018
May 08 17:20:22 up2 kernel: Workqueue: events sof_probe_work [snd_sof]
May 08 17:20:22 up2 kernel: RIP: 0010:snd_sof_load_firmware_raw+0x7b/0x190 [snd_sof]
May 08 17:20:22 up2 kernel: Code: 85 c0 0f 84 2b 01 00 00 48 8b 13 48 89 c6 48 89 ef e8 d9 e3 d5 da 41 89 c4 85 c0 0f 88 24 92 00 00 0f 1f 4>
May 08 17:20:22 up2 kernel: RSP: 0018:ffffa8db80223e18 EFLAGS: 00010246
May 08 17:20:22 up2 kernel: RAX: 0000000000000000 RBX: ffff928dee8a6828 RCX: 0000000000000000
May 08 17:20:22 up2 kernel: RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff928dfbd98820
May 08 17:20:22 up2 kernel: RBP: ffff928deebe7c28 R08: 00000001383405ae R09: ffffffff9c8c1374
May 08 17:20:22 up2 kernel: R10: 0000000000000393 R11: 000000000002cfc0 R12: 00000000fffffffe
May 08 17:20:22 up2 kernel: R13: ffff928deebe7c28 R14: 0000000000000000 R15: ffff928de8d79bc0
May 08 17:20:22 up2 kernel: FS: 0000000000000000(0000) GS:ffff928dfbd80000(0000) knlGS:0000000000000000
May 08 17:20:22 up2 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
May 08 17:20:22 up2 kernel: CR2: 0000000000000000 CR3: 0000000178fb4000 CR4: 00000000003406e0
May 08 17:20:22 up2 kernel: Call Trace:
May 08 17:20:22 up2 kernel: sof_probe_work+0x81/0x220 [snd_sof]
May 08 17:20:22 up2 kernel: process_one_work+0x1d2/0x3a0
May 08 17:20:22 up2 kernel: worker_thread+0x45/0x3c0
May 08 17:20:22 up2 kernel: kthread+0xf6/0x130
May 08 17:20:22 up2 kernel: ? process_one_work+0x3a0/0x3a0
May 08 17:20:22 up2 kernel: ? kthread_park+0x80/0x80
May 08 17:20:22 up2 kernel: ret_from_fork+0x35/0x40
May 08 17:20:22 up2 kernel: Modules linked in: snd_hda_codec_hdmi snd_soc_hdac_hdmi snd_soc_dmic snd_sof_pci snd_sof_intel_hda_common snd_so>
May 08 17:20:22 up2 kernel: CR2: 0000000000000000
May 08 17:20:22 up2 kernel: ---[ end trace 33baffe6567bf588 ]---
May 08 17:20:22 up2 kernel: RIP: 0010:snd_sof_load_firmware_raw+0x7b/0x190 [snd_sof]
May 08 17:20:22 up2 kernel: Code: 85 c0 0f 84 2b 01 00 00 48 8b 13 48 89 c6 48 89 ef e8 d9 e3 d5 da 41 89 c4 85 c0 0f 88 24 92 00 00 0f 1f 4>
May 08 17:20:22 up2 kernel: RSP: 0018:ffffa8db80223e18 EFLAGS: 00010246
May 08 17:20:22 up2 kernel: RAX: 0000000000000000 RBX: ffff928dee8a6828 RCX: 0000000000000000
May 08 17:20:22 up2 kernel: RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff928dfbd98820
May 08 17:20:22 up2 kernel: RBP: ffff928deebe7c28 R08: 00000001383405ae R09: ffffffff9c8c1374
May 08 17:20:22 up2 kernel: R10: 0000000000000393 R11: 000000000002cfc0 R12: 00000000fffffffe
May 08 17:20:22 up2 kernel: R13: ffff928deebe7c28 R14: 0000000000000000 R15: ffff928de8d79bc0
May 08 17:20:22 up2 kernel: FS: 0000000000000000(0000) GS:ffff928dfbd80000(0000) knlGS:0000000000000000
May 08 17:20:22 up2 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
May 08 17:20:22 up2 kernel: CR2: 0000000000000000 CR3: 0000000178fb4000 CR4: 00000000003406e0

ktrzcinx requested review from dbaluta, juimonen, keyonjie, kv2019i, lgirdwood, lyakh, plbossart and ranj063 as code owners March 18, 2020 07:07

lyakh reviewed Mar 19, 2020
View reviewed changes

Comment threadsound/soc/sof/loader.c

ktrzcinx force-pushed the ext-mnf-pr-fix branch from 0a74307 to 26d3965Compare March 19, 2020 08:07

ktrzcinx requested a review from lyakh March 19, 2020 08:12

lyakh previously approved these changes Mar 19, 2020
View reviewed changes

plbossart requested changes Mar 20, 2020
View reviewed changes

Comment threadsound/soc/sof/loader.c

ktrzcinx dismissed lyakh’s stale review via 09a2f2cMarch 20, 2020 07:48

ktrzcinx force-pushed the ext-mnf-pr-fix branch from 26d3965 to 09a2f2cCompare March 20, 2020 07:48

ktrzcinx mentioned this pull request Mar 20, 2020
Create extended manifest thesofproject/sof#2427
Merged

kv2019i requested changes Mar 20, 2020
View reviewed changes

ktrzcinx force-pushed the ext-mnf-pr-fix branch 2 times, most recently from 3fb1427 to ed50f53Compare March 23, 2020 08:09

kv2019i requested review from kv2019i, lyakh and plbossart March 23, 2020 11:28

kv2019i approved these changes Mar 23, 2020
View reviewed changes

plbossart previously approved these changes Mar 23, 2020
View reviewed changes

lyakh previously approved these changes Mar 24, 2020
View reviewed changes

lyakh requested changes Apr 28, 2020
View reviewed changes

Comment threadsound/soc/sof/loader.c Outdated
Comment threadsound/soc/sof/loader.c Outdated
Comment threadsound/soc/sof/loader.c Outdated

ktrzcinx force-pushed the ext-mnf-pr-fix branch from e85a0a9 to bed6720Compare April 29, 2020 11:07

ktrzcinx requested review from kv2019i and lyakh April 29, 2020 11:07

ktrzcinx force-pushed the ext-mnf-pr-fix branch from bed6720 to d0b2382Compare April 29, 2020 18:50

lyakh previously approved these changes Apr 30, 2020
View reviewed changes

ktrzcinx added 2 commits April 30, 2020 08:59

ASoC: SOF: ext_manifest: parse firmware version
cd98918
The firmware version can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ktrzcinx dismissed lyakh’s stale review via 713cd69April 30, 2020 07:03

ktrzcinx force-pushed the ext-mnf-pr-fix branch 2 times, most recently from 713cd69 to 91ffbbfCompare April 30, 2020 07:05

ktrzcinx requested a review from lyakh April 30, 2020 08:31

kv2019i previously approved these changes Apr 30, 2020
View reviewed changes

lyakh reviewed May 4, 2020
View reviewed changes

ktrzcinx added 2 commits May 4, 2020 08:27

ASoC: SOF: ext_manifest: parse windows
589d9d9
The window description can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ASoC: SOF: ext_manifest: parse compiler version
e0cfedd
The compiler version and description can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ktrzcinx dismissed kv2019i’s stale review via e0cfeddMay 4, 2020 06:34

ktrzcinx force-pushed the ext-mnf-pr-fix branch from 91ffbbf to e0cfeddCompare May 4, 2020 06:34

ktrzcinx requested review from kv2019i and lyakh May 4, 2020 06:39

kv2019i approved these changes May 4, 2020
View reviewed changes

lyakh approved these changes May 5, 2020
View reviewed changes

kv2019i requested review from ranj063 and removed request for ranj063 May 5, 2020 12:07

ranj063 approved these changes May 5, 2020
View reviewed changes

ranj063 merged commit c08157b into thesofproject:topic/sof-devMay 5, 2020

ktrzcinx mentioned this pull request May 13, 2020
fixup! ASoC: SOF: Introduce extended manifest #2095
Merged

ktrzcinx mentioned this pull request Oct 6, 2020
Remove data duplication between mailbox and extended manifest - save memory thesofproject/sof#3497
Merged

Uh oh!

Conversation

ktrzcinx commented Mar 18, 2020

Uh oh!

kv2019i commented Mar 18, 2020

Uh oh!

plbossart commented Mar 18, 2020

Uh oh!

Uh oh!

ranj063 commented Mar 19, 2020

Uh oh!

Uh oh!

kv2019i commented Mar 20, 2020

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019iMar 20, 2020

Choose a reason for hiding this comment

Uh oh!

lyakhMar 23, 2020

Choose a reason for hiding this comment

Uh oh!

ktrzcinxMar 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kv2019i left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kv2019i commented Mar 24, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

lyakhMay 4, 2020

Choose a reason for hiding this comment

Uh oh!

ktrzcinxMay 4, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kv2019i commented May 5, 2020

Uh oh!

marc-hb commented May 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kv2019i left a comment •
edited
Loading

kv2019i commented Mar 24, 2020•
edited
Loading

ktrzcinxMay 4, 2020•
edited
Loading