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.

Sign up for GitHub

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

Jump to bottom

The TLV header numid is ignored in bytes control set from user space and random value seen in header in get #5282

Open
singalsu opened this issue Jan 3, 2025 · 2 comments · May be fixed by #5281
Open

The TLV header numid is ignored in bytes control set from user space and random value seen in header in get #5282

singalsu opened this issue Jan 3, 2025 · 2 comments · May be fixed by #5281

Comments

@singalsu
Copy link

singalsu commented Jan 3, 2025

When working with SOF issue thesofproject/sof#9092 with new version of sof-ctl in thesofproject/sof#9760 I noticed that in IPC4 systems the get response returns in TLV header a IPC4 component id that is from user space point-of-view a random number. It's assigned by kernel on the fly to a running number, so it varies even with same pipeline component instance in different use scenarios with other pipelines.

It would be more clear if the get response would be the same as in data for control set. We use in the blobs as legacy from IPC3 the value SOF_CTRL_CMD_BINARY.

If the response data from get is predictable and same as in set we can directly without need to ignore the header compare the data from get operation to blobs stored into the system to see what it is, e.g. /usr/share/alsa/ucm2/blobs/sof/ipc4/eq_iir/pass.blob.

There is similar issue in control set. The numid is checked in IPC3 kernel to be SOF_CTRL_CMD_BINARY to pass. In IPC4 systems the value is not checked.
@singalsu
Copy link
Author

singalsu commented Jan 3, 2025

@ujfalusi @ranj063 I created this issue to help decide if we want to change the IPC4 bytes control. From my point of view, the current way is not a blocker, but somewhat confusing.

@ujfalusi
Copy link
Collaborator

ujfalusi commented Jan 3, 2025

@singalsu, thank you, I agree that the current way of passing comp_id as numid to user space and ignoring the numid from user space is not good. The comp_id is kernel internal running number used per swidgets, it can be different based on different factors.

I will update the kernel patch to use SOF_CTRL_CMD_BINARY for numid.

ujfalusi added a commit to ujfalusi/sof-linux that referenced this issue Jan 3, 2025
@ujfalusi
ASoC: SOF: ipc4-control: Use SOF_CTRL_CMD_BINARY as numid for bytes_ext
afa5ed6 
The header.numid is set to scontrol->comp_id in bytes_ext_get and it is
ignored during bytes_ext_put.
The use of comp_id is not quite great as it is kernel internal
identification number.

Set the header.numid to SOF_CTRL_CMD_BINARY during get and validate the
numid during put to provide consistent and compatible identification
number as IPC3.

For IPC4 existing tooling also ignored the numid but with the use of
SOF_CTRL_CMD_BINARY the different handling of the blobs can be dropped,
providing better user experience.

Reported-by: Seppo Ingalsuo <[email protected]>
Closes: thesofproject#5282
Fixes: a062c88 ("ASoC: SOF: ipc4-control: Add support for bytes control get and put")
Cc: [email protected]
Signed-off-by: Peter Ujfalusi <[email protected]>
@ujfalusi ujfalusi linked a pull request Jan 3, 2025 that will close this issue
Open
ujfalusi added a commit to ujfalusi/sof-linux that referenced this issue Jan 8, 2025
@ujfalusi
ASoC: SOF: ipc4-control: Use SOF_CTRL_CMD_BINARY as numid for bytes_ext
adb6154 
The header.numid is set to scontrol->comp_id in bytes_ext_get and it is
ignored during bytes_ext_put.
The use of comp_id is not quite great as it is kernel internal
identification number.

Set the header.numid to SOF_CTRL_CMD_BINARY during get and validate the
numid during put to provide consistent and compatible identification
number as IPC3.

For IPC4 existing tooling also ignored the numid but with the use of
SOF_CTRL_CMD_BINARY the different handling of the blobs can be dropped,
providing better user experience.

Reported-by: Seppo Ingalsuo <[email protected]>
Closes: thesofproject#5282
Fixes: a062c88 ("ASoC: SOF: ipc4-control: Add support for bytes control get and put")
Cc: [email protected]
Signed-off-by: Peter Ujfalusi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ASoC: SOF: ipc4-control: Use SOF_CTRL_CMD_BINARY as numid for bytes_ext ujfalusi/sof-linux
2 participants
@ujfalusi @singalsu