Re: [PATCH] media: venus: hfi: fix error handling in hfi_sys_init_done()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jul 9, 2017 at 3:49 PM, Stanimir Varbanov
<stanimir.varbanov@xxxxxxxxxx> wrote:
> Hi Rob,
>
> On 07/09/2017 04:19 PM, Rob Clark wrote:
>> Not entirely sure what triggers it, but with venus build as kernel
>> module and in initrd, we hit this crash:
>
> Is it happens occasionally or everytime in the initrd? And also with
> your patch it will bail out on venus_probe, does it crash again on next
> venus_probe?

seems to happen every time.. (module is ending up in initrd, but not
sure if fw is.. which might be triggering this?)

I could not boot successfully without this patch, but otoh I haven't
yet tried if venus actually works after boot.

BR,
-R

>>
>>   Unable to handle kernel paging request at virtual address ffff80003c039000
>>   pgd = ffff00000a14f000
>>   [ffff80003c039000] *pgd=00000000bd9f7003, *pud=00000000bd9f6003, *pmd=00000000bd9f0003, *pte=0000000000000000
>>   Internal error: Oops: 96000007 [#1] SMP
>>   Modules linked in: qcom_wcnss_pil(E+) crc32_ce(E) qcom_common(E) venus_core(E+) remoteproc(E) snd_soc_msm8916_digital(E) virtio_ring(E) cdc_ether(E) snd_soc_lpass_apq8016(E) snd_soc_lpass_cpu(E) snd_soc_apq8016_sbc(E) snd_soc_lpass_platform(E) v4l2_mem2mem(E) virtio(E) snd_soc_core(E) ac97_bus(E) snd_pcm_dmaengine(E) snd_seq(E) leds_gpio(E) videobuf2_v4l2(E) videobuf2_core(E) snd_seq_device(E) sndi_pcm(E) videodev(E) media(E) nvmem_qfprom(E) msm(E) snd_timer(E) snd(E) soundcore(E) spi_qup(E) mdt_loader(E) qcom_tsens(E) qcom_spmi_temp_alarm(E) nvmem_core(E) msm_rng(E) uas(E) usb_storage(E) dm9601(E) usbnet(E) mii(E) mmc_block(E) adv7511(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) qcom_spmi_vadc(E) qcom_vadc_common(PE) industrialio(E) pinctrl_spmi_mpp(E)
>>    pinctrl_spmi_gpio(E) rtc_pm8xxx(E) clk_smd_rpm(E) sdhci_msm(E) sdhci_pltfm(E) qcom_smd_regulator(E) drm(E) smd_rpm(E) qcom_spmi_pmic(E) regmap_spmi(E) ci_hdrc_msm(E) ci_hdrc(E) usb3503(E) extcon_usb_gpio(E) phy_msm_usb(E) udc_core(E) qcom_hwspinlock(E) extcon_core(E) ehci_msm(E) i2c_qup(E) sdhci(E) mmc_core(E) spmi_pmic_arb(E) spmi(E) qcom_smd(E) smsm(E) rpmsg_core(E) smp2p(E) smem(E) hwspinlock_core(E) gpio_keys(E)
>>   CPU: 2 PID: 551 Comm: irq/150-venus Tainted: P            E   4.12.0+ #1625
>>   Hardware name: qualcomm dragonboard410c/dragonboard410c, BIOS 2017.07-rc2-00144-ga97bdbdf72-dirty 07/08/2017
>>   task: ffff800037338000 task.stack: ffff800038e00000
>>   PC is at hfi_sys_init_done+0x64/0x140 [venus_core]
>>   LR is at hfi_process_msg_packet+0xcc/0x1e8 [venus_core]
>>   pc : [<ffff00000118b384>] lr : [<ffff00000118c11c>] pstate: 20400145
>>   sp : ffff800038e03c60
>>   x29: ffff800038e03c60 x28: 0000000000000000
>>   x27: 00000000000df018 x26: ffff00000118f4d0
>>   x25: 0000000000020003 x24: ffff80003a8d3010
>>   x23: ffff00000118f760 x22: ffff800037b40028
>>   x21: ffff8000382981f0 x20: ffff800037b40028
>>   x19: ffff80003c039000 x18: 0000000000000020
>>   x17: 0000000000000000 x16: ffff800037338000
>>   x15: ffffffffffffffff x14: 0000001000000014
>>   x13: 0000000100001007 x12: 0000000100000020
>>   x11: 0000100e00000000 x10: 0000000000000001
>>   x9 : 0000000200000000 x8 : 0000001400000001
>>   x7 : 0000000000001010 x6 : 0000000000000148
>>   x5 : 0000000000001009 x4 : ffff80003c039000
>>   x3 : 00000000cd770abb x2 : 0000000000000042
>>   x1 : 0000000000000788 x0 : 0000000000000002
>>   Process irq/150-venus (pid: 551, stack limit = 0xffff800038e00000)
>>   Call trace:
>>   [<ffff00000118b384>] hfi_sys_init_done+0x64/0x140 [venus_core]
>>   [<ffff00000118c11c>] hfi_process_msg_packet+0xcc/0x1e8 [venus_core]
>>   [<ffff00000118a2b4>] venus_isr_thread+0x1b4/0x208 [venus_core]
>>   [<ffff00000118e750>] hfi_isr_thread+0x28/0x38 [venus_core]
>>   [<ffff000008161550>] irq_thread_fn+0x30/0x70
>>   [<ffff0000081617fc>] irq_thread+0x14c/0x1c8
>>   [<ffff000008105e68>] kthread+0x138/0x140
>>   [<ffff000008083590>] ret_from_fork+0x10/0x40
>>   Code: 52820125 52820207 7a431820 54000249 (b9400263)
>>   ---[ end trace c963460f20a984b6 ]---
>>
>> The problem is that in the error case, we've incremented the data ptr
>> but not decremented rem_bytes, and keep reading (presumably garbage)
>> until eventually we go beyond the end of the buffer.
>>
>> Instead, on first error, we should probably just bail out.  Other
>> option is to increment read_bytes by sizeof(u32) before the switch,
>> rather than only accounting for the ptype header in the non-error
>> case.  Note that in this case it is HFI_ERR_SYS_INVALID_PARAMETER,
>> ie. an unrecognized/unsupported parameter, so interpreting the next
>> word as a property type would be bogus.  The other error cases are
>> due to truncated buffer, so there isn't likely to be anything valid
>> to interpret in the remainder of the buffer.  So just bailing seems
>> like a reasonable solution.
>
> I have a WIP patch which rewrite the message parsing, it would be nice
> if I can reproduce this crash.
>
>>
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>> ---
>>  drivers/media/platform/qcom/venus/hfi_msgs.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
>> index debf80a92797..4190825b20a1 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
>> @@ -239,11 +239,12 @@ static void hfi_sys_init_done(struct venus_core *core, struct venus_inst *inst,
>>                       break;
>>               }
>>
>> -             if (!error) {
>> -                     rem_bytes -= read_bytes;
>> -                     data += read_bytes;
>> -                     num_properties--;
>> -             }
>> +             if (error)
>> +                     break;
>> +
>> +             rem_bytes -= read_bytes;
>> +             data += read_bytes;
>> +             num_properties--;
>>       }
>>
>>  err_no_prop:
>>
>
> --
> regards,
> Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux