| 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
| ||
|
Message-ID: <eddc2967-8355-b64a-79d8-6c1cda1bc732@suse.de>
Date: Wed, 3 Nov 2021 13:57:47 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Javier Martinez Canillas <javierm@...hat.com>,
linux-kernel@...r.kernel.org
Cc: David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel.vetter@...ll.ch>,
dri-devel@...ts.freedesktop.org,
Gurchetan Singh <gurchetansingh@...omium.org>,
Gerd Hoffmann <kraxel@...hat.com>,
amd-gfx@...ts.freedesktop.org,
VMware Graphics <linux-graphics-maintainer@...are.com>,
Peter Robinson <pbrobinson@...il.com>,
Neal Gompa <ngompa13@...il.com>,
Dave Airlie <airlied@...hat.com>,
Ben Skeggs <bskeggs@...hat.com>,
Michel Dänzer <michel@...nzer.net>,
Hans de Goede <hdegoede@...hat.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
nouveau@...ts.freedesktop.org,
virtualization@...ts.linux-foundation.org,
Pekka Paalanen <pekka.paalanen@...labora.com>,
"Pan, Xinhui" <Xinhui.Pan@....com>,
spice-devel@...ts.freedesktop.org,
Alex Deucher <alexander.deucher@....com>,
intel-gfx@...ts.freedesktop.org,
Christian König <christian.koenig@....com>
Subject: Re: [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to
drm_modeset_disabled()
Hi
Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
> This function is used by some DRM drivers to determine if the "nomodeset"
> kernel command line parameter was set and prevent these drivers to probe.
>
> But the function name is quite confusing and does not reflect what really
> the drivers are testing when calling it. Use a better naming now that it
> is part of the DRM subsystem.
>
> Also, vgacon_text_force() is guarded by #ifdef CONFIG_VGA_CONSOLE already
> so there is no need to do the same when calling the function.
>
> Suggested-by: Suggested-by: Daniel Vetter <daniel.vetter@...ll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> ---
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/drm_nomodeset.c | 16 ++++++++--------
> drivers/gpu/drm/i915/i915_module.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
> drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
> drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
> drivers/gpu/drm/tiny/bochs.c | 2 +-
> drivers/gpu/drm/tiny/cirrus.c | 2 +-
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 +---
> drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> include/drm/drm_mode_config.h | 4 ++--
> 14 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2680a2aaa877..f7bd2616cf23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
> {
> int r;
>
> - if (vgacon_text_force()) {
> + if (drm_modeset_disabled()) {
> DRM_ERROR("amdgpu kernel modesetting disabled.\n");
Please remove all such error messages from drivers.
drm_modeset_disabled() should print a unified message instead.
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 048be607b182..6706050414c3 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -232,7 +232,7 @@ static struct pci_driver ast_pci_driver = {
>
> static int __init ast_init(void)
> {
> - if (vgacon_text_force() && ast_modeset == -1)
> + if (drm_modeset_disabled() && ast_modeset == -1)
> return -EINVAL;
>
> if (ast_modeset == 0)
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> index 1ac9a8d5a8fe..dfc8b30f0625 100644
> --- a/drivers/gpu/drm/drm_nomodeset.c
> +++ b/drivers/gpu/drm/drm_nomodeset.c
> @@ -3,17 +3,17 @@
> #include <linux/module.h>
> #include <linux/types.h>
>
> -static bool vgacon_text_mode_force;
> +static bool drm_nomodeset;
>
> -bool vgacon_text_force(void)
> +bool drm_modeset_disabled(void)
I suggest to rename this function to drm_check_modeset() and have it
return a negative errno code on failure. This gives maximum flexibility
and reduces errors in drivers. Right now the drivers return something
like -EINVAL, which seems wrong. Returning -ENODEV seems more appropriate.
> {
> - return vgacon_text_mode_force;
> + return drm_nomodeset;
> }
> -EXPORT_SYMBOL(vgacon_text_force);
> +EXPORT_SYMBOL(drm_modeset_disabled);
>
> -static int __init text_mode(char *str)
> +static int __init disable_modeset(char *str)
> {
> - vgacon_text_mode_force = true;
> + drm_nomodeset = true;
>
> pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
> pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> @@ -22,5 +22,5 @@ static int __init text_mode(char *str)
> return 1;
> }
>
> -/* force text mode - used by kernel modesetting */
> -__setup("nomodeset", text_mode);
> +/* Disable kernel modesetting */
> +__setup("nomodeset", disable_modeset);
> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
> index 14a59226519d..3e5531040e4d 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -29,7 +29,7 @@ static int i915_check_nomodeset(void)
> if (i915_modparams.modeset == 0)
> use_kms = false;
>
> - if (vgacon_text_force() && i915_modparams.modeset == -1)
> + if (drm_modeset_disabled() && i915_modparams.modeset == -1)
> use_kms = false;
>
> if (!use_kms) {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 685e766db6a4..7ee87564bade 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -377,7 +377,7 @@ static struct pci_driver mgag200_pci_driver = {
>
> static int __init mgag200_init(void)
> {
> - if (vgacon_text_force() && mgag200_modeset == -1)
> + if (drm_modeset_disabled() && mgag200_modeset == -1)
> return -EINVAL;
>
> if (mgag200_modeset == 0)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 029997f50d1a..903d0e626954 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1321,7 +1321,7 @@ nouveau_drm_init(void)
> nouveau_display_options();
>
> if (nouveau_modeset == -1) {
> - if (vgacon_text_force())
> + if (drm_modeset_disabled())
> nouveau_modeset = 0;
> }
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 3cd6bd9f059d..e4ab16837fad 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -294,7 +294,7 @@ static struct drm_driver qxl_driver = {
>
> static int __init qxl_init(void)
> {
> - if (vgacon_text_force() && qxl_modeset == -1)
> + if (drm_modeset_disabled() && qxl_modeset == -1)
> return -EINVAL;
>
> if (qxl_modeset == 0)
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9b606c1b11ec..36c8dac68cca 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -636,7 +636,7 @@ static struct pci_driver radeon_kms_pci_driver = {
>
> static int __init radeon_module_init(void)
> {
> - if (vgacon_text_force() && radeon_modeset == -1) {
> + if (drm_modeset_disabled() && radeon_modeset == -1) {
> DRM_INFO("VGACON disable radeon kernel modesetting.\n");
> radeon_modeset = 0;
> }
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 04333f78be55..59189f7c1840 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -718,7 +718,7 @@ static struct pci_driver bochs_pci_driver = {
>
> static int __init bochs_init(void)
> {
> - if (vgacon_text_force() && bochs_modeset == -1)
> + if (drm_modeset_disabled() && bochs_modeset == -1)
> return -EINVAL;
>
> if (bochs_modeset == 0)
> diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> index 8bd674f0d682..fcf98379c641 100644
> --- a/drivers/gpu/drm/tiny/cirrus.c
> +++ b/drivers/gpu/drm/tiny/cirrus.c
> @@ -635,7 +635,7 @@ static struct pci_driver cirrus_pci_driver = {
>
> static int __init cirrus_init(void)
> {
> - if (vgacon_text_force())
> + if (drm_modeset_disabled())
> return -EINVAL;
> return pci_register_driver(&cirrus_pci_driver);
> }
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index e6d983121d0b..09356dbd69b2 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -192,10 +192,8 @@ static const struct drm_driver driver = {
>
> static int __init vbox_init(void)
> {
> -#ifdef CONFIG_VGA_CONSOLE
> - if (vgacon_text_force() && vbox_modeset == -1)
> + if (drm_modeset_disabled() && vbox_modeset == -1)
> return -EINVAL;
> -#endif
>
> if (vbox_modeset == 0)
> return -EINVAL;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index cd4c170236f1..d96797d70fae 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -103,7 +103,7 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
> struct drm_device *dev;
> int ret;
>
> - if (vgacon_text_force() && virtio_gpu_modeset == -1)
> + if (drm_modeset_disabled() && virtio_gpu_modeset == -1)
> return -EINVAL;
>
> if (virtio_gpu_modeset == 0)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index fcc4b5a7f639..22dab9beea03 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1650,7 +1650,7 @@ static int __init vmwgfx_init(void)
> {
> int ret;
>
> - if (vgacon_text_force())
> + if (drm_modeset_disabled())
> return -EINVAL;
>
> ret = pci_register_driver(&vmw_pci_driver);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e1d2042a7b77..a5a2dc02e892 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -970,9 +970,9 @@ void drm_mode_config_reset(struct drm_device *dev);
> void drm_mode_config_cleanup(struct drm_device *dev);
>
> #ifdef CONFIG_VGA_CONSOLE
> -extern bool vgacon_text_force(void);
> +extern bool drm_modeset_disabled(void);
> #else
> -static inline bool vgacon_text_force(void) { return false; }
> +static inline bool drm_modeset_disabled(void) { return false; }
> #endif
>
> #endif
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)
Powered by blists - more mailing lists