linux-kernel - Re: [EXT] [PATCH 09/15] qed: use new module_firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e10b611e-f925-f12d-bcd2-ba60d86dd8d0@marvell.com>
Date:   Tue, 12 May 2020 19:23:28 +0300
From:   Igor Russkikh <irusskikh@...vell.com>
To:     Luis Chamberlain <mcgrof@...nel.org>
CC:     <jeyu@...nel.org>, <akpm@...ux-foundation.org>, <arnd@...db.de>,
        <rostedt@...dmis.org>, <mingo@...hat.com>, <aquini@...hat.com>,
        <cai@....pw>, <dyoung@...hat.com>, <bhe@...hat.com>,
        <peterz@...radead.org>, <tglx@...utronix.de>,
        <gpiccoli@...onical.com>, <pmladek@...e.com>, <tiwai@...e.de>,
        <schlad@...e.de>, <andriy.shevchenko@...ux.intel.com>,
        <keescook@...omium.org>, <daniel.vetter@...ll.ch>,
        <will@...nel.org>, <mchehab+samsung@...nel.org>,
        <kvalo@...eaurora.org>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Ariel Elior <aelior@...vell.com>,
        GR-everest-linux-l2 <GR-everest-linux-l2@...vell.com>
Subject: Re: [EXT] [PATCH 09/15] qed: use new module_firmware_crashed()


>> So I think its not a good place to insert this call.
>> Its hard to find exact good place to insert it in qed.
> 
> Is there a way to check if what happened was indeed a fw crash?

Our driver has two firmwares (slowpath and fastpath).
For slowpath firmware the way to understand it crashed is to observe command
response timeout. This is in qed_mcp.c, around "The MFW failed to respond to
command" traceout.

For fastpath this is tricky, think you may leave the above place as the only
place to invoke module_firmware_crashed()

> 
>> One more thing is that AFAIU taint flag gets permanent on kernel, but
> for
>> example our device can recover itself from some FW crashes, thus it'd be
>> transparent for user.
> 
> Similar things are *supposed* to recoverable with other device, however
> this can also sometimes lead to a situation where devices are not usable
> anymore, and require a full driver unload / load.
> 
>> Whats the logical purpose of module_firmware_crashed? Does it mean fatal
>> unrecoverable error on device?
> 
> Its just to annotate on the module and kernel that this has happened.
> 
> I take it you may agree that, firmware crashing *often* is not good
> design,
> and these issues should be reported to / fixed by vendors. In cases
> where driver bugs are reported it is good to see if a firmware crash has
> happened before, so that during analysis this is ruled out.

Probably, but still I see some misalignment here, in sense that taint is about
the kernel state, not about a hardware state indication.

devlink health could really be a much better candidate for such things.

Regards
  Igor