linux-kernel - Re: [PATCH] drm/panfrost: Mark simple

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: <f672e7460c92bc9e0c195804f7e99d0b@manjaro.org>
Date: Wed, 03 Jul 2024 14:42:37 +0200
From: Dragan Simic <dsimic@...jaro.org>
To: dri-devel@...ts.freedesktop.org
Cc: boris.brezillon@...labora.com, robh@...nel.org, steven.price@....com,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 airlied@...il.com, daniel@...ll.ch, linux-kernel@...r.kernel.org, Diederik
 de Haas <didi.debian@...ow.org>, Furkan Kardame <f.kardame@...jaro.org>,
 stable@...r.kernel.org
Subject: Re: [PATCH] drm/panfrost: Mark simple_ondemand governor as softdep

Hello everyone,

On 2024-06-17 22:17, Dragan Simic wrote:
> Panfrost DRM driver uses devfreq to perform DVFS, while using 
> simple_ondemand
> devfreq governor by default.  This causes driver initialization to fail 
> on
> boot when simple_ondemand governor isn't built into the kernel 
> statically,
> as a result of the missing module dependency and, consequently, the 
> required
> governor module not being included in the initial ramdisk.  Thus, let's 
> mark
> simple_ondemand governor as a softdep for Panfrost, to have its kernel 
> module
> included in the initial ramdisk.
> 
> This is a rather longstanding issue that has forced distributions to 
> build
> devfreq governors statically into their kernels, [1][2] or has forced 
> users
> to introduce some unnecessary workarounds. [3]
> 
> For future reference, not having support for the simple_ondemand 
> governor in
> the initial ramdisk produces errors in the kernel log similar to these 
> below,
> which were taken from a Pine64 RockPro64:
> 
>   panfrost ff9a0000.gpu: [drm:panfrost_devfreq_init [panfrost]]
> *ERROR* Couldn't initialize GPU devfreq
>   panfrost ff9a0000.gpu: Fatal error during GPU init
>   panfrost: probe of ff9a0000.gpu failed with error -22
> 
> Having simple_ondemand marked as a softdep for Panfrost may not resolve 
> this
> issue for all Linux distributions.  In particular, it will remain 
> unresolved
> for the distributions whose utilities for the initial ramdisk 
> generation do
> not handle the available softdep information [4] properly yet.  
> However, some
> Linux distributions already handle softdeps properly while generating 
> their
> initial ramdisks, [5] and this is a prerequisite step in the right 
> direction
> for the distributions that don't handle them properly yet.
> 
> [1] 
> https://gitlab.manjaro.org/manjaro-arm/packages/core/linux/-/blob/linux61/config?ref_type=heads#L8180
> [2] https://salsa.debian.org/kernel-team/linux/-/merge_requests/1066
> [3] https://forum.pine64.org/showthread.php?tid=15458
> [4] 
> https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
> [5] 
> https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4489054668ad
> 
> Cc: Diederik de Haas <didi.debian@...ow.org>
> Cc: Furkan Kardame <f.kardame@...jaro.org>
> Cc: stable@...r.kernel.org
> Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
> Signed-off-by: Dragan Simic <dsimic@...jaro.org>

Just checking, could this patch be accepted, please?  The Lima 
counterpart
has already been accepted. [6]

The approach in this patch is far from perfect, but it's still fine 
until
there's a better solution, such as harddeps.  I'll continue my research
about the possibility for introducing harddeps, which would hopefully
replace quite a few instances of the softdep (ab)use that already extend
rather far.  For example, have a look at the commit d5178578bcd4 (btrfs:
directly call into crypto framework for checksumming) [7] and the lines
containing MODULE_SOFTDEP() at the very end of fs/btrfs/super.c. [8]

If a filesystem driver can rely on the (ab)use of softdeps, which may be
fragile or seen as a bit wrong, I think we can follow the same approach,
at least until a better solution is available.

[6] 
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0c94f58cef319ad054fd909b3bf4b7d09c03e11c
[7] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5178578bcd4
[8] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/super.c#n2593

> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index ef9f6c0716d5..149737d7a07e 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -828,3 +828,4 @@ module_platform_driver(panfrost_driver);
>  MODULE_AUTHOR("Panfrost Project Developers");
>  MODULE_DESCRIPTION("Panfrost DRM Driver");
>  MODULE_LICENSE("GPL v2");
> +MODULE_SOFTDEP("pre: governor_simpleondemand");