linux-kernel - Re: [PATCH] drm/panthor: Add defer probe for firmware load

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] [day] [month] [year] [list]

Message-ID: <87y1918psd.fsf@minerva.mail-host-address-is-not-set>
Date: Thu, 25 Apr 2024 16:45:06 +0200
From: Javier Martinez Canillas <javierm@...hat.com>
To: Steven Price <steven.price@....com>, Andy Yan <andyshrk@....com>,
 boris.brezillon@...labora.com
Cc: daniel@...ll.ch, airlied@...il.com, liviu.dudau@....com,
 maarten.lankhorst@...ux.intel.com, tzimmermann@...e.de,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, Andy Yan
 <andy.yan@...k-chips.com>
Subject: Re: [PATCH] drm/panthor: Add defer probe for firmware load

Steven Price <steven.price@....com> writes:

> Hi Javier,
>
> On 25/04/2024 10:22, Javier Martinez Canillas wrote:
>> Steven Price <steven.price@....com> writes:
>> 
>> Hello Steven,
>> 
>>> On 13/04/2024 12:49, Andy Yan wrote:
>>>> From: Andy Yan <andy.yan@...k-chips.com>
>>>>
>>>> The firmware in the rootfs will not be accessible until we
>>>> are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until
>>>> that point.
>>>> This let the driver can load firmware when it is builtin.
>>>
>>> The usual solution is that the firmware should be placed in the
>>> initrd/initramfs if the module is included there (or built-in). The same
>>> issue was brought up regarding the powervr driver:
>>>
>>> https://lore.kernel.org/dri-devel/20240109120604.603700-1-javierm@redhat.com/T/
>>>
>>> I'm not sure if that ever actually reached a conclusion though. The
>>> question was deferred to Greg KH but I didn't see Greg actually getting
>>> involved in the thread.
>>>
>> 
>> Correct, there was not conclusion reached in that thread.
>
> So I think we need a conclusion before we start applying point fixes to
> individual drivers.
>

Agreed.

[...]

>> 
>> In the thread you referenced I suggested to add that logic in request_firmware()
>> (or add a new request_firmware_defer() helper function) that changes the request
>> firmare behaviour to return -EPROBE_DEFER instead of -ENOENT.
>
> That would seem like a good feature if it's agreed that deferring on
> request_firmware is a good idea.
>

Yeah. I didn't attempt to type that patch because didn't get an answer
from Greg and didn't want to spent the time writing and testing a patch
to just be nacked.

>> Since as you mentioned, this isn't specific to panthor and an issue that I also
>> faced with the powervr driver.
>
> I'm not in any way against the idea of deferring the probe until the
> firmware is around - indeed it seems like a very sensible idea in many
> respects. But I don't want panthor to be 'special' in this way.
>
> If the consensus is that the firmware should live with the module (i.e.
> either both in the initramfs or both in the rootfs) then the code is
> fine as it is. That seemed to be the view of Sima in that thread and
> seems reasonable to me - why put the .ko in the initrd if you can't
> actually use it until the rootfs comes along?
>

That's indeed a sensible position for me as well and is what I answered to
the user who reported the powervr issue.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat