rimage: manifest: uuid: llext: Get loadable module UUID from uuid-registry and unify module manifest uuid type #9793
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change the type of the UUID parameter in the ipc4_get_drv function to avoid unnecessary typecasting. There are currently two ways to store UUIDs in SOF: as a byte array or as a sof_uuid structure. The ipc4_get_drv function compares UUIDs using the memcmp function, so it is not dependent on the type of the argument passed. This change is intended to prepare the code for standardizing the UUID format across the project. Signed-off-by: Adrian Warecki <[email protected]>
Change the uuid parameter type to void * in the parse_uuid function to make it independent of the UUID type and avoid unnecessary type casting. This change is intended to prepare the code for standardizing the UUID format across the project. Signed-off-by: Adrian Warecki <[email protected]>
Some identifiers are taken from the module manifest. Since the module manifest structure (sof_man_module) is marked as packed, the pointer to the sof_uuid structure may not be properly aligned. Mark the sof_uuid structure as packed to avoid possible problems with accessing fields of this structure from unaligned addresses. Signed-off-by: Adrian Warecki <[email protected]>
Separate the SOF_REG_UUID macro to allow use it to initialize variables with a value from the register. Signed-off-by: Adrian Warecki <[email protected]>
Remove the reference to the first element of the array storing the uuid to prepare the code for changing the uuid type from an array to a struct. Signed-off-by: Adrian Warecki <[email protected]>
Add a & symbol for getting the address of a variable to prepare the code for changing the uuid type from an array to a structure. Signed-off-by: Adrian Warecki <[email protected]>
softwarecki
requested review from
lyakh,
lgirdwood,
abonislawski,
kv2019i,
marcinszkudlinski,
jxstelter,
tmleman and
serhiy-katsyuba-intel
softwarecki
requested review from
a team,
tlissows,
singalsu,
fkwasowi,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
Change the UUID storage type in the module manifest to a sof-compatible structure to enable retrieval of the UUIDs from the registry. Obtain the UUIDs of loadable llext modules from the uuid-registry. Signed-off-by: Adrian Warecki <[email protected]>
lyakh
approved these changes
Jan 24, 2025
There was a problem hiding this comment.
very good and welcome change! Doing those UUIDs manually is a pain and error-prone! A bit "too late" should I say after I've converted almost all of them, but better late than never! :-) And it still seems to work after this change:
[ 1349.578565] <wrn> ipc: ipc4_get_drv: get_drv(): the provided UUID (b36ee4da-006f-47f9-a06d-fecbe2d8b6ce) can't be found!
[ 1349.583076] <inf> llext: llext_load: Loaded extension DRC
just would be good to rearrange a couple of changes, otherwise no objections!
| @@ -998,13 +999,13 @@ const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) | |||
| #endif | |||
| } | |||
| /* Check already registered components */ | |||
| drv = ipc4_get_drv(mod->uuid); | |||
| drv = ipc4_get_drv(&mod->uuid); | |||
There was a problem hiding this comment.
currently .uuid in struct sof_man_module is an array, so yes, both these expressions generate the same result, and you probably want to change .uuid to a structure, which is why you need this change. But let's do that in that commit where you do it, not here.
| @@ -641,7 +641,7 @@ int lib_manager_register_module(const uint32_t component_id) | |||
| } | |||
|
|
|||
| mod = (struct sof_man_module *)((const uint8_t *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); | |||
| const struct sof_uuid *uid = (struct sof_uuid *)&mod->uuid[0]; | |||
| const struct sof_uuid *uid = (struct sof_uuid *)&mod->uuid; | |||
There was a problem hiding this comment.
you could move those two hunks to this commit as "preparation" or move all three changes to where you change to the structure
serhiy-katsyuba-intel
approved these changes
Jan 24, 2025
marcinszkudlinski
approved these changes
Jan 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rimage: Change the UUID storage type in the module manifest to a sof-compatible structure to enable retrieval of the UUIDs from the registry.
llext: Obtain the UUIDs of loadable modules from the uuid-registry.
BTW: The MICSEL module had an incorrect byte order in its uuid.