linux-kernel - Re: [RFC PATCH] drm/aperture: Add param to disable conflicting framebuffers removal

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: <352163b2-2946-aec8-16b4-fd6f01373ff2@redhat.com>
Date:   Fri, 22 Oct 2021 17:16:16 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Neal Gompa <ngompa13@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Peter Robinson <pbrobinson@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [RFC PATCH] drm/aperture: Add param to disable conflicting
 framebuffers removal

Hello Neal,

Thanks for your feedback.

On 10/22/21 16:56, Neal Gompa wrote:
> On Fri, Oct 22, 2021 at 10:40 AM Javier Martinez Canillas
> <javierm@...hat.com> wrote:
>>
>> The simpledrm driver allows to use the frame buffer that was set-up by the
>> firmware. This gives early video output before the platform DRM driver is
>> probed and takes over.
>>
>> But it would be useful to have a way to disable this take over by the real
>> DRM drivers. For example, there may be bugs in the DRM drivers that could
>> cause the display output to not work correctly.
>>
>> For those cases, it would be good to keep the simpledrm driver instead and
>> at least get a working display as set-up by the firmware.
>>
>> Let's add a drm.remove_fb boolean kernel command line parameter, that when
>> set to false will prevent the conflicting framebuffers to being removed.
>>
>> Since the drivers call drm_aperture_remove_conflicting_framebuffers() very
>> early in their probe callback, this will cause the drivers' probe to fail.
>>
>> Thanks to Neal Gompa for the suggestion and Thomas Zimmermann for the idea
>> on how this could be implemented.
>>
>> Suggested-by: Neal Gompa <ngompa13@...il.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
>> ---
>> Hello,
>>
>> I'm sending this as an RFC because I wasn't sure about the correct name for
>> this module parameter, and also if 'remove_fb=0' is intitutive or instead a
>> parameter that's enabled is preferred (i.e: 'disable_fb_removal=1').
>>
> 
> In general, I think the patch is fine, but it might make sense to name
> the parameter after the *effect* rather than the *action*. That is,
> the effect of this option is that we don't probe and hand over to a
> more appropriate hardware DRM driver.
> 
> Since the effect (in DRM terms) is that we don't use platform DRM
> modules, perhaps we could name the option one of these:
> 
> * drm.noplatformdrv
> * drm.simpledrv
> * drm.force_simple
>

or maybe drm.disable_handover ? Naming is hard...
 
> I'm inclined to say we should use the drm.* namespace for the cmdline
> option because that makes it clear what subsystem it affects. The
> legacy "nomodeset" option kind of sucked because it didn't really tell
> you what that meant, and I'd rather not repeat that mistake.
>

Yes, agreed. In fact, that is the case for this patch since the param is
in the drm module. I just forgot to mention the namespace in the last
paragraph of the comment.

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat