[BUG] Inconsistent component ID assignment

[aiChaoSONG]

The primary message for init module instance structure is listed below:

		struct {
			/**< module id */
			uint32_t module_id          : 16;
			/**< instance id */
			uint32_t instance_id        : 8;
			/**< ModuleMsg::INIT_INSTANCE */
			uint32_t type               : 5;
			/**< Msg::MSG_REQUEST */
			uint32_t rsp                : 1;
			/**< Msg::MODULE_MSG */
			uint32_t msg_tgt            : 1;
			uint32_t _reserved_0        : 1;
		} r;

module id takes 16 bits, and instance_id takes next 8 bits.

Here is how we assign component ID;

	comp_id = IPC4_COMP_ID(module_init->primary.r.module_id,
			       module_init->primary.r.instance_id);

and the ID macros:

#define IPC4_COMP_ID(x, y)	((x) << 16 | (y))
#define IPC4_MOD_ID(x) ((x) >> 16)
#define IPC4_INST_ID(x)	((x) & 0xffff)
#define IPC4_SRC_QUEUE_ID(x)	(((x) >> 16) & 0xffff)
#define IPC4_SINK_QUEUE_ID(x)	((x) & 0xffff)

module_id<<16|instance_id doesn't comply to the ipc structure, which make it hard to do message cross check between mtrace and linux kernel message, and also leave a 8 bit hole in the 32bit comp_id, bad.

[lgirdwood]

@abonislawski @marcinszkudlinski can someone clarify the correct usage ? Thanks !

[aiChaoSONG]

@lgirdwood When the code for IPC4 is added, this issue appears. As ipc_get_comp_by_id() assumes the id is unique among component and pipeline, the author use module_id<<16|instance_id to make it unique (a trick), this is based on the module_id is never zero, because module_id==0 is taken by BringUp and BaseFw.

ipc_get_comp_by_id() is also used by IPC3 and many common codes, it is not that easy to correct this in a short time.

It never hurts if we mark this as won't fix.

[lyakh]

In fact in general using bitfields in ABIs seems to be a bad idea, e.g. https://stackoverflow.com/questions/6043483/why-bit-endianness-is-an-issue-in-bitfields . So eventually we might want to replace all such bitfields structures with well-defined for each endianness bitwise access operations

[aiChaoSONG]

fixed by #8057, close