ASoC: SOF: loader: fix kernel oops on firmware boot failure - #1174
Conversation
When we fail to boot the firmware, we encounter a kernel oops in hda_dsp_get_registers(), which is called conditionally in hda_dsp_dump() when the sdev_>boot_complete flag is set. Setting this flag _after_ dumping the data fixes the issue and does not change the programming flow. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
| snd_sof_dsp_dbg_dump(sdev, SOF_DBG_REGS | SOF_DBG_MBOX | | ||
| SOF_DBG_TEXT | SOF_DBG_PCI); | ||
| /* after this point FW_READY msg should be ignored */ | ||
| sdev->boot_complete = true; |
There was a problem hiding this comment.
@plbossart just wondering why do we need to set this to true at all in case of FW boot failure?
There was a problem hiding this comment.
@kv2019i Can you chime in, this change was yours initially I believe and IIRC it was intentional. I don't recall the reason...
There was a problem hiding this comment.
@plbossart i looked up the commit that added the change a69270d. I think @kv2019i hit this race condition with the QEMU DSP. But nevertheless, this change looks fine to me.
There was a problem hiding this comment.
@plbossart @ranj063 Ack on that. Without that you can have the case where we consider FW loading to have failed to a timeout and have freed, or are freeing related resources -- but then we get the late FW_READY and this will make the driver go crazy. Thus the fix.
|
The change itself here is harmless. With respect to the commit @kv2019i made, I think it's better to use other solution like this: And then in the place boot failed here, set boot_complete to BOOT_FAILED, in IPC handling, handle fw_ready message only at BOOT_ONGOING stage. |
|
@keyonjie wrote:
Hmm, this probably would be cleaner solution (easier to understand for the casual readong.. in failed case, the boot did not really complete, so it's misleading), agreed. |
keyonjie
left a comment
keyonjie
left a comment
There was a problem hiding this comment.
It's fine to fix the issue firstly.
@aiChaoSONG is working on the clean up for boot_complete.
please file an enhancement so that we can track progress, thanks. |
Sure, here it is: #1179 |
4 participants
When we fail to boot the firmware, we encounter a kernel oops in
hda_dsp_get_registers(), which is called conditionally in
hda_dsp_dump() when the sdev_>boot_complete flag is set.
Setting this flag after dumping the data fixes the issue and does
not change the programming flow.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com