linux-kernel - Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB

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: <393ca142-5226-4779-a963-c34fb0464c59@app.fastmail.com>
Date:   Sun, 02 Jul 2023 00:24:49 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Javier Martinez Canillas" <javierm@...hat.com>,
        linux-kernel@...r.kernel.org
Cc:     "Thomas Zimmermann" <tzimmermann@...e.de>,
        "Geert Uytterhoeven" <geert@...ux-m68k.org>,
        "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
        "Borislav Petkov" <bp@...en8.de>,
        "Daniel Vetter" <daniel@...ll.ch>,
        "Dave Hansen" <dave.hansen@...ux.intel.com>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "H. Peter Anvin" <hpa@...or.com>, "Helge Deller" <deller@....de>,
        "Ingo Molnar" <mingo@...hat.com>,
        "Randy Dunlap" <rdunlap@...radead.org>,
        "Sam Ravnborg" <sam@...nborg.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org,
        x86@...nel.org
Subject: Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

On Sat, Jul 1, 2023, at 23:44, Javier Martinez Canillas wrote:
> Currently the CONFIG_FB option has to be enabled even if no legacy fbdev
> drivers are needed (e.g: only to have support for framebuffer consoles).
>
> The DRM subsystem has a fbdev emulation layer, but depends on CONFIG_FB
> and so it can only be enabled if that dependency is enabled as well.
>
> That means fbdev drivers have to be explicitly disabled if users want to
> enable CONFIG_FB, only to use fbcon and/or the DRM fbdev emulation layer.
>
> This patch introduces a non-visible CONFIG_FB_CORE symbol that could be
> enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
> allowing CONFIG_FB to be disabled (and automatically disabling all the
> fbdev drivers).
>
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> ---

I found two more things now:

> 
> +menuconfig FB_CORE
> +	tristate "Core support for frame buffer devices"
> +

This is not actually a hidden option, since you left the prompt
after the 'tristate' keyword. There is also no pointn in having
it as a menu, just use the simpler

config FB_CORE
        tristate

or (as in my other email)

config FB_CORE
       def_tristate FB || (DRM && DRM_FBDEV_EMULATION)


> @@ -44,7 +54,7 @@ menuconfig FB
> 
>  config FIRMWARE_EDID
>  	bool "Enable firmware EDID"
> -	depends on FB
> +	depends on FB_CORE
>  	help
>  	  This enables access to the EDID transferred from the firmware.
>  	  On the i386, this is from the Video BIOS. Enable this if DDC/I2C
> @@ -59,7 +69,7 @@ config FIRMWARE_EDID
> 
>  config FB_DEVICE
>  	bool "Provide legacy /dev/fb* device"
> -	depends on FB
> +	select FB_CORE
>  	default y
>  	help
>  	  Say Y here if you want the legacy /dev/fb* device file and

These are now the only user visible sub-options when CONFIG_FB is
disabled. I missed FIRMWARE_EDID earlier, but this also looks like
it can clearly be left as depending on FB since nothing else calls
fb_firmware_edid. In fact, it looks like all of fbmon.c could be
left out since none of its exported symbols are needed for DRM.

That would leave CONFIG_FB_DEVICE as the only user visible option
for DRM-only configs, which is slightly odd for the menuconfig,
so I still wonder if that could be done differently.

Is there actually a point in configurations for kernels with FB=y,
DRM=n and FB_DEVICE=n? If we don't expect that to be a useful
configuration, an easier way would be to have CONFIG_FB turn it
on implicitly and instead have a user-visible Kconfig option
below CONFIG_DRM_FBDEV_EMULATION that allows controlling the
creation of /dev/fb*.

     Arnd