On Wed, 2019-03-20 at 09:45 +0100, Guennadi Liakhovetski wrote:
Hi,
This code in hda_dsp_ipc_get_reply() in hda-ipc.c looks suspicious to me:
hdr = msg->msg_data; if (hdr->cmd == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE)) { ... } else { /* get IPC reply from DSP in the mailbox */ sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply)); } if (reply.error < 0) { size = sizeof(reply); ret = reply.error; } else { ... }
/* read the message */ if (msg->msg_data && size > 0) sof_mailbox_read(sdev, sdev->host_box.offset, msg->reply_data, size);
So, as far as I understand, first of all the first part of the "if (msg->msg_data && ...)" test is always true, since we've already dereferenced "hdr" above.
Secondly if reply contains a negative error the reply struct will be read from the mailbox again, instead of just using "memcpy(msg->reply_data, &reply, sizeof(reply));" to copy it? The same pattern is also repeated in other IPC implementations.
Best to send a fix.
Thanks
Liam
Thanks Guennadi _______________________________________________ Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware