Repository navigation
[BUG] ext manifest has extra element #3507
Description
Describe the bug
Working on thesofproject/linux#2297.
Parsing CAVS EXT MANIFEST, it is supposed to parse total 3 elems from ext_man_cavs_config_data but total 5 of elems are parsed. There are unnecessary extra elems. It causes warnings.
Should EXT_MAN_CAVS_CONFIG_LAST_ELEM be up to date with last element? like below?
/* EXT_MAN_ELEM_PLATFORM_CONFIG_DATA elements identificators */
enum cavs_config_elem_type {
EXT_MAN_CAVS_CONFIG_LPRO = 1,
EXT_MAN_CAVS_CONFIG_OUTBOX_SIZE,
EXT_MAN_CAVS_CONFIG_INBOX_SIZE,
EXT_MAN_CAVS_CONFIG_LAST_ELEM = EXT_MAN_CAVS_CONFIG_INBOX_SIZE, /**< keep up to date with last element */
};
/* EXT_MAN_ELEM_PLATFORM_CONFIG_DATA elements */
struct ext_man_cavs_config_data {
struct ext_man_elem_header hdr;
struct config_elem elems[EXT_MAN_CAVS_CONFIG_LAST_ELEM];
} __packed;
And even with this fix, first 0 is unavoidable. as we start from 1. (short log is below) one warning is always printed in kernel side.
Kernel folks are very picky about unnecessary warnings. Any suggestion about this also?
How about define uninitialized elem_type,
EXT_MAN_CAVS_CONFIG_UNINIT = 0,
To Reproduce
Need to test with thesofproject/linux#2297.
Reproduction Rate
100%
Expected behavior
Only 3 elements are parsed
Impact
What impact does this issue have on your progress (e.g., annoyance, showstopper)
Environment
SOF main branch + #2957 and linux pr 2297
Screenshots or console output
[ 1.582426] sof-audio-pci 0000:00:1f.3: hdr->type: 6, hdr->size = 48, elem_num=5
[ 1.582428] sof-audio-pci 0000:00:1f.3: FW clock config: LPRO
[ 1.582430] sof-audio-pci 0000:00:1f.3: unsupported token type: 2 value=4096
[ 1.582431] sof-audio-pci 0000:00:1f.3: unsupported token type: 3 value=8192
[ 1.582433] sof-audio-pci 0000:00:1f.3: unsupported token type: 0 value=0
[ 1.582434] sof-audio-pci 0000:00:1f.3: unsupported token type: 0 value=0
Activity
fredoh9 commentedon Oct 8, 2020
In second thought, I don't remember why we have to start this from 1. I kind of remember it caused some issues but I'm not sure that concern is still valid. Can we start this enum from 0 also?
EXT_MAN_CAVS_CONFIG_LPRO = 0,ranj063 commentedon Oct 8, 2020
It needs to start from one for the exac reason that we have static sizes array and the elems in the static array will be initialized to 0. Then you wouldnt know if it is a real element or the default element in the array?
ranj063 commentedon Oct 8, 2020
besides if you only parse the exact number of elems based on size, you wont have this problem isnt it?
fredoh9 commentedon Oct 8, 2020
Ah, I now remember the discussion why it starts from 1.
lyakh commentedon Oct 9, 2020
Aren't there also maybe some other fields that we can check to verify, that the element is valid? Or you could make the array size
array[LAST - 1];? But as discussed in thesofproject/linux#2297 I don't find the idea of the size of an array being exactly equal to the current number of possible elements, good.ktrzcinx commentedon Oct 9, 2020
What profit it has above just skipping zero filled elements ? Maybe we should just explicit mark EXT_MAN_CONFIG_EMPTY.
it's rather array capacity, defined to workaround xcc problems with static flex arrays
lgirdwood commentedon Oct 9, 2020
This is intentional as it means we can append new manifest elems without breaking the kernel (kernel just ignores the new feature and in this case prints a warning). The last elem ID does NOT have to be ABI aligned between FW and kernel, as long as the array elem count is correct.
xiulipan commentedon Oct 20, 2020
@fredoh9 I also have a RFC #3527 to add a FIRST enum. This will keep minor when we add new enum.
lgirdwood commentedon Oct 20, 2020
@fredoh9 just fyi @ktrzcinx and @xiulipan are syncing on fix for this given discussion today on call.
github-actions commentedon Jun 20, 2025
This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.