[WIP] ext_manifest: Add UUID dictionary

[ktrzcinx]

Dictionary for tranformation UUID value to UUID entry address used in
FW runtime should be available for driver.

Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com

[lgirdwood]

Why does the driver need this information, please state the driver uses cases in the commit message.

[lgirdwood]

@ktrzcinx @mmaka1 driver is not going to doing any dictionary lookups. It's going to handle the UUIDs as opaque passthrough data only. i.e for topology creation, the driver will read UUID from topology file and pass straight to FW to do the UUID lookup and create the component.

[paulstelian97]

@ktrzcinx @mmaka1 driver is not going to doing any dictionary lookups. It's going to handle the UUIDs as opaque passthrough data only. i.e for topology creation, the driver will read UUID from topology file and pass straight to FW to do the UUID lookup and create the component.

Unless firmware will store the 32-byte-long UUIDs itself (and I'm not sure how it would do that on the severely memory-constraint platforms), the kernel would have to perform a dictionary lookup (or the topology store the result of the lookup, but that would be too messed up)

[ktrzcinx]

This PR need changes from thesofproject/rimage#16 and thesofproject/rimage#17 to end up work

[lgirdwood]

@ktrzcinx @mmaka1 driver is not going to doing any dictionary lookups. It's going to handle the UUIDs as opaque passthrough data only. i.e for topology creation, the driver will read UUID from topology file and pass straight to FW to do the UUID lookup and create the component.

Unless firmware will store the 32-byte-long UUIDs itself (and I'm not sure how it would do that on the severely memory-constraint platforms), the kernel would have to perform a dictionary lookup (or the topology store the result of the lookup, but that would be too messed up)

I agree having the 32 byte data in FW (as RO data) is not ideal, but userspace is using this as reference so we use it in FW too.

@ktrzcinx If we need to down convert then the UUID design is broken, i.e. we should have been only using a 32bit UUID in FW, driver and user to begin with. It's not like we have a large namespace or are likely to even overflow 32bits.

[paulstelian97]

@ktrzcinx @mmaka1 driver is not going to doing any dictionary lookups. It's going to handle the UUIDs as opaque passthrough data only. i.e for topology creation, the driver will read UUID from topology file and pass straight to FW to do the UUID lookup and create the component.

Unless firmware will store the 32-byte-long UUIDs itself (and I'm not sure how it would do that on the severely memory-constraint platforms), the kernel would have to perform a dictionary lookup (or the topology store the result of the lookup, but that would be too messed up)

I agree having the 32 byte data in FW (as RO data) is not ideal, but userspace is using this as reference so we use it in FW too.

@ktrzcinx If we need to down convert then the UUID design is broken, i.e. we should have been only using a 32bit UUID in FW, driver and user to begin with. It's not like we have a large namespace or are likely to even overflow 32bits.

The biggest issue with the approach of only using 32-bit IDs is going to be conflicts, especially given that in the future SOF itself isn't going to be the only authority that is handing out IDs. Do we really want to always have an ABI change every time a third party component is to be integrated, even if that component is going to be vendor specific? Or will we just reserve some of the namespace for vendor specific and have the topologies be incompatible between vendors (possibly referencing the wrong component even)?

The 32-byte UUID with an in-kernel translation layer is the best compromise I can think of, given the issues in the previous paragraph as well. We'd essentially go back to the old design if we do 32-bit IDs (UUID is 32 bytes by definition)

[lgirdwood]

@ktrzcinx If we need to down convert then the UUID design is broken, i.e. we should have been only using a 32bit UUID in FW, driver and user to begin with. It's not like we have a large namespace or are likely to even overflow 32bits.

The biggest issue with the approach of only using 32-bit IDs is going to be conflicts, especially given that in the future SOF itself isn't going to be the only authority that is handing out IDs. Do we really want to always have an ABI change every time a third party component is to be integrated, even if that component is going to be vendor specific? Or will we just reserve some of the namespace for vendor specific and have the topologies be incompatible between vendors (possibly referencing the wrong component even)?

I don't see any big risk of collisions if the IDs are random, it's n/2^32 where n is the number of components. FWiw, I have more chance of winning the UK lottery :)

The 32-byte UUID with an in-kernel translation layer is the best compromise I can think of, given the issues in the previous paragraph as well. We'd essentially go back to the old design if we do 32-bit IDs (UUID is 32 bytes by definition)

The kernel should not really be tracking FW context in order to translate between 32 byte and 32 bit UUIDs. Any translation would have to be deterministic and static to avoid tracking / coherency commitments.

It's also really not clear to me where will will be referencing the FW by UUID. Both kcontriol and PCMs are named/numbered. Topology can used this in creation, OR it could use the "compressed UUID" directly, again this needs a deterministsic UUID to compressed ID.

[keyonjie]

@paulstelian97 for integration of those vendor specific component, we don't necessarily define new component type for it, instead, we can use the new defined generic process component type together with flavor/subtype (transparent to the driver) then no code change needed in the driver, please refer to:
https://github.com/orgs/thesofproject/teams/sof-developers/discussions/32

[paulstelian97]

@ktrzcinx If we need to down convert then the UUID design is broken, i.e. we should have been only using a 32bit UUID in FW, driver and user to begin with. It's not like we have a large namespace or are likely to even overflow 32bits.

The biggest issue with the approach of only using 32-bit IDs is going to be conflicts, especially given that in the future SOF itself isn't going to be the only authority that is handing out IDs. Do we really want to always have an ABI change every time a third party component is to be integrated, even if that component is going to be vendor specific? Or will we just reserve some of the namespace for vendor specific and have the topologies be incompatible between vendors (possibly referencing the wrong component even)?

I don't see any big risk of collisions if the IDs are random, it's n/2^32 where n is the number of components. FWiw, I have more chance of winning the UK lottery :)

It's similar to a birthday attack, we want to avoid two matching each other, the chances for that are around n/2^17 or something (not really, but it takes 2^16 + change to get to 50% chance of a collision). If you believe 2^16 for 50% chance of collision is fine then I concur.

The 32-byte UUID with an in-kernel translation layer is the best compromise I can think of, given the issues in the previous paragraph as well. We'd essentially go back to the old design if we do 32-bit IDs (UUID is 32 bytes by definition)

The kernel should not really be tracking FW context in order to translate between 32 byte and 32 bit UUIDs. Any translation would have to be deterministic and static to avoid tracking / coherency commitments.

It's also really not clear to me where will will be referencing the FW by UUID. Both kcontriol and PCMs are named/numbered. Topology can used this in creation, OR it could use the "compressed UUID" directly, again this needs a deterministsic UUID to compressed ID.

Maybe we could just manually allocate the types, and the subtype is up to the vendor that owns the type. I don't know, doing this will remove basically every advantage UUID is touted to give.

[ktrzcinx]

1. 32bit value of UUID is not random, please look at tools: logger: Add LDC dump option #2627 description, there are list of 32bit UUID key/address and 32 bytes UUID value for each component. So they practically can't overflow, but even so, then linker warn about it.

2. We already use 32bit UUID key/address in FW in trace system, ie. in struct log_entry_header definition, where user see full 32 bytes UUID value in logs, to follow convention - after translation in logger. Struct sof_uuid entries are located in non loadable .static_uuids section, so there is no access to them in runtime.

3. We already have 54 UUID entries in source code, 54 times 28 (32 bytes - 4 bytes) gives 1.5k flash memory save / power optimization, and it will grow with number of UUID entries.

4. Iterating and comparing over 32bit values are much simpler than 32 bytes one, the same is for sending through IPC - single instruction vs loop in each place - so it's even more time performance, flash size, power reduction, less error prone optimization

[kv2019i]

@ktrzcinx wrote:

3. We already have 54 UUID entries in source code, 54 times 28 (32 bytes - 4 bytes) gives 1.5k flash memory save / power optimization, and it will grow with number of UUID entries.

Thanks for providing numbers. It seems this is at heart of this discussion -- is this saving worth the operational complexity or not? At this point, we have multiple new DSP processing components stuck in the devel pipeline (#2802, #2925), so one way or another we'd need to get forward.

To balance the numbers, as per #769 , only requirement for uuid usage in kernel is topology load and that only affects COMP_NEW IPC.
In SOF master that means 21 entries (I'm not counting "component"). So that's 21*(32-4)=588 bytes.
Further, if very constrained on resources, I would expect not all components to be built to the image, so practical number would be lower.

Is that 588+ bytes too much or not, that's the core issue.

For tracing, I understand the compressed form, because you need to identify all components, so the count can be higher and you may need to do high-volume trace output, so saving bits makes sense.

PS I thought about segmenting the audio uuids in a well known subsegment. E.g. reserve a 20bit subspace of full 128bit uuid for topology-visible public components, and map this with a known formula to the 32bit dictionary. But, but, this would have too high probability of collisions and complicate the operations side too much (of course, if we make adding components really, really hard, that sure helps with the collisions ;)).

[lgirdwood]

@ktrzcinx I had to double check, but the UUID is 32 bytes for ASCII and 16 bytes for binary. Since we already have 4 bytes for "type" then it should be a 12 byte increment (after type is removed). i.e. 12 * 54 = 648 extra bytes in RO data. If this becomes a problem in the future we can always move to IMR with other infrequently accessed data/text.

[ktrzcinx]

This solution was rejected, full UUID will be passed during building topology.