linux-kernel - Re: [PATCH 00/11] drm: Restore helper usability

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]

Date: Mon, 22 Apr 2024 20:11:19 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Arnd Bergmann <arnd@...db.de>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	Dave Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, 
	Linux-Renesas <linux-renesas-soc@...r.kernel.org>, Masahiro Yamada <masahiroy@...nel.org>, 
	linux-kbuild <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH 00/11] drm: Restore helper usability

Hi Jani,

CC kbuild

On Mon, Apr 22, 2024 at 7:00 PM Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@...db.de> wrote:
> > I'm not sure where this misunderstanding comes from, as you
> > seem to be repeating the same incorrect assumption about
> > how select works that Maxime wrote in his changelog. To clarify,
> > this works exactly as one would expect:
> >
> > config HELPER_A
> >        tristate
> >
> > config HELPER_B
> >        tristate
> >        select HELPER_A
> >
> > config DRIVER
> >        tristate "Turn on the driver and the helpers it uses"
> >        select HELPER_B # this recursively selects HELPER_A
> >
> > Whereas this one is broken:
> >
> > config FEATURE_A
> >        tristate "user visible if I2C is enabled"
> >        depends on I2C
> >
> > config HELPER_B
> >        tristate # hidden
> >        select FEATURE_A
> >
> > config DRIVER
> >        tristate "This driver is broken if I2C is disabled"
> >        select HELPER_B
>
> This case is really what I was referring to, although I was sloppy with
> words there. I understand that select does work recursively for selects.
>
> >>   There is no end to this, it just goes on and on, as the
> >>   dependencies of the selected symbols change over time. Often the
> >>   selects require unintuitive if patterns that are about the
> >>   implementation details of the symbol being selected.
> >
> > Agreed, that is the problem I frequently face with drivers/gpu/drm,
> > and most of the time it can only be solved by rewriting the whole
> > system to not select user-visible symbol at all.
> >
> > Using 'depends on' by itself is unfortunately not enough to
> > avoid /all/ the problems. See e.g. today's failure
> >
> > config DRM_DISPLAY_HELPER
> >        tristate "DRM Display Helpers"
> >        default y
> >
> > config DRM_DISPLAY_DP_HELPER
> >        bool "DRM DisplayPort Helpers"
> >        depends on DRM_DISPLAY_HELPER
> >
> > config DRM_PANEL_LG_SW43408
> >        tristate "LG SW43408 panel"
> >        depends on DRM_DISPLAY_DP_HELPER
> >
> > This version is still broken for DRM_DISPLAY_HELPER=m,
> > DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because
> > the dependency on the bool symbol is not enough to
> > ensure that DRM_DISPLAY_HELPER is also built-in, so you
> > still need explicit dependencies on both
> > DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users.
> >
> > This can be solved by making DRM_DISPLAY_DP_HELPER a
> > tristate symbol and adjusting the #ifdef checks and
> > Makefile logic accordingly, which is exactly what you'd
> > need to do to make it work with 'select' as well.
>
> So bool is kind of problematic for depends on and select even when it's
> not really used for describing builtin vs. no, but rather yes vs. no?

Yes, the underlying issue is that bool is used for two different things:
  A. To enable a driver module that can be only built-in,
  B. To enable an option or feature of a driver or subsystem.

Without this distinction, dependencies cannot be auto-propagated 100%
correctly.  Fixing that would require introducing a third type (and possibly
renaming the existing ones to end up with 3 good names).

Actually two types could work:
  1. driver,
  2. option,
as case A is just a driver that can only be built-in (i.e. "depends on y",
which is similar to the behavior with CONFIG_MODULES=n).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds