| 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: <2698c680-6d05-f58d-d7c2-ea76aeb0bb47@suse.de>
Date: Fri, 5 Nov 2021 10:39:09 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Jani Nikula <jani.nikula@...ux.intel.com>,
Javier Martinez Canillas <javierm@...hat.com>,
linux-kernel@...r.kernel.org
Cc: Michel Dänzer <michel@...nzer.net>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Peter Robinson <pbrobinson@...il.com>,
Pekka Paalanen <pekka.paalanen@...labora.com>,
Alex Deucher <alexander.deucher@....com>,
Ben Skeggs <bskeggs@...hat.com>, Chia-I Wu <olvaffe@...il.com>,
Christian König <christian.koenig@....com>,
Daniel Vetter <daniel@...ll.ch>,
Dave Airlie <airlied@...hat.com>,
David Airlie <airlied@...ux.ie>,
Gerd Hoffmann <kraxel@...hat.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Gurchetan Singh <gurchetansingh@...omium.org>,
Hans de Goede <hdegoede@...hat.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
"Pan, Xinhui" <Xinhui.Pan@....com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
VMware Graphics <linux-graphics-maintainer@...are.com>,
Zack Rusin <zackr@...are.com>, amd-gfx@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
linux-fbdev@...r.kernel.org, nouveau@...ts.freedesktop.org,
spice-devel@...ts.freedesktop.org,
virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 2/2] drm: Move nomodeset kernel parameter to the DRM
subsystem
Hi
Am 05.11.21 um 10:22 schrieb Jani Nikula:
> On Fri, 05 Nov 2021, Thomas Zimmermann <tzimmermann@...e.de> wrote:
>> Hi
>>
>> Am 04.11.21 um 17:07 schrieb Javier Martinez Canillas:
>>> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
>>> but the exported vgacon_text_force() symbol is only used by DRM drivers.
>>>
>>> It makes much more sense for the parameter logic to be in the subsystem
>>> of the drivers that are making use of it.
>>>
>>> Let's move the vgacon_text_force() function and related logic to the DRM
>>> subsystem. While doing that, rename the function to drm_check_modeset()
>>> which better reflects what the function is really used to test for.
>>>
>>> Suggested-by: Daniel Vetter <daniel.vetter@...ll.ch>
>>> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Conditionally build drm_nomodeset.o if CONFIG_VGA_CONSOLE is set.
>>> - Squash patches to move nomodeset logic to DRM and do the renaming.
>>> - Name the function drm_check_modeset() and make it return -ENODEV.
>>>
>>> drivers/gpu/drm/Makefile | 2 ++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
>>> drivers/gpu/drm/ast/ast_drv.c | 1 -
>>> drivers/gpu/drm/drm_drv.c | 9 +++++----
>>> drivers/gpu/drm/drm_nomodeset.c | 26 +++++++++++++++++++++++++
>>> drivers/gpu/drm/i915/i915_module.c | 2 --
>>> drivers/gpu/drm/mgag200/mgag200_drv.c | 1 -
>>> drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
>>> drivers/gpu/drm/qxl/qxl_drv.c | 1 -
>>> drivers/gpu/drm/radeon/radeon_drv.c | 1 -
>>> drivers/gpu/drm/tiny/bochs.c | 1 -
>>> drivers/gpu/drm/tiny/cirrus.c | 1 -
>>> drivers/gpu/drm/vboxvideo/vbox_drv.c | 1 -
>>> drivers/gpu/drm/virtio/virtgpu_drv.c | 1 -
>>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 -
>>> drivers/video/console/vgacon.c | 21 --------------------
>>> include/drm/drm_mode_config.h | 6 ++++++
>>> include/linux/console.h | 6 ------
>>> 18 files changed, 39 insertions(+), 44 deletions(-)
>>> create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>>>
>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>> index 1c41156deb5f..c74810c285af 100644
>>> --- a/drivers/gpu/drm/Makefile
>>> +++ b/drivers/gpu/drm/Makefile
>>> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
>>>
>>> obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>>>
>>> +obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>>> +
>>
>> This now depends on the VGA textmode console. Even if you have no VGA
>> console, you'd want drm_nomodeset.o. Simpledrm might be built-in and can
>> provide graphics. Non-PC systems don't even have a VGA device.
>
> This was discussed in an earlier version, which had this builtin but the
> header still had a stub for CONFIG_VGA_CONSOLE=n.
>
>> I think we really want a separate boolean config option that gets
>> selected by CONFIG_DRM.
>
> Perhaps that should be a separate change on top.
Sure, make it a separate patch.
We want to make this work on ARM systems. I even have a request to
replace offb on Power architecture by simpledrm. So the final config has
to be system agnostic.
Best regards
Thomas
>
> BR,
> Jani.
>
>>
>>
>>> drm_cma_helper-y := drm_gem_cma_helper.o
>>> obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 7fde40d06181..b4b6993861e6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -31,7 +31,6 @@
>>> #include "amdgpu_drv.h"
>>>
>>> #include <drm/drm_pciids.h>
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pm_runtime.h>
>>> #include <linux/vga_switcheroo.h>
>>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>>> index 802063279b86..6222082c3082 100644
>>> --- a/drivers/gpu/drm/ast/ast_drv.c
>>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>>> @@ -26,7 +26,6 @@
>>> * Authors: Dave Airlie <airlied@...hat.com>
>>> */
>>>
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>>
>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 3fb567d62881..80b85b8ea776 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -986,12 +986,13 @@ EXPORT_SYMBOL(drm_dev_set_unique);
>>> */
>>> int drm_drv_enabled(const struct drm_driver *driver)
>>> {
>>> - if (vgacon_text_force()) {
>>> + int ret;
>>> +
>>> + ret = drm_check_modeset();
>>> + if (ret)
>>> DRM_INFO("%s driver is disabled\n", driver->name);
>>> - return -ENODEV;
>>> - }
>>>
>>> - return 0;
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(drm_drv_enabled);
>>>
>>> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
>>> new file mode 100644
>>> index 000000000000..6683e396d2c5
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/drm_nomodeset.c
>>> @@ -0,0 +1,26 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/types.h>
>>> +
>>> +static bool drm_nomodeset;
>>> +
>>> +int drm_check_modeset(void)
>>> +{
>>> + return drm_nomodeset ? -ENODEV : 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_check_modeset);
>>> +
>>> +static int __init disable_modeset(char *str)
>>> +{
>>> + 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");
>>> + pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
>>
>> I'd update this text to be less sensational.
>>
>>> +
>>> + return 1;
>>> +}
>>> +
>>> +/* 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 45cb3e540eff..c890c1ca20c4 100644
>>> --- a/drivers/gpu/drm/i915/i915_module.c
>>> +++ b/drivers/gpu/drm/i915/i915_module.c
>>> @@ -4,8 +4,6 @@
>>> * Copyright © 2021 Intel Corporation
>>> */
>>>
>>> -#include <linux/console.h>
>>> -
>>
>> These changes should be in patch 1?
>>
>>> #include "gem/i915_gem_context.h"
>>> #include "gem/i915_gem_object.h"
>>> #include "i915_active.h"
>>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
>>> index 2a581094ba2b..8e000cac11ba 100644
>>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
>>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
>>> @@ -6,7 +6,6 @@
>>> * Dave Airlie
>>> */
>>>
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> #include <linux/vmalloc.h>
>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
>>> index 8844d3602d87..bd1456521b7c 100644
>>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>>> @@ -22,7 +22,6 @@
>>> * Authors: Ben Skeggs
>>> */
>>>
>>> -#include <linux/console.h>
>>> #include <linux/delay.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 3ac2ef2bf545..ff070ac76111 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -29,7 +29,6 @@
>>>
>>> #include "qxl_drv.h"
>>>
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> #include <linux/vgaarb.h>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
>>> index 56d688c04346..f59cc971ec95 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>>> @@ -31,7 +31,6 @@
>>>
>>>
>>> #include <linux/compat.h>
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pm_runtime.h>
>>> #include <linux/vga_switcheroo.h>
>>> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
>>> index ee6b1ff9128b..6e9a31f1a0f3 100644
>>> --- a/drivers/gpu/drm/tiny/bochs.c
>>> +++ b/drivers/gpu/drm/tiny/bochs.c
>>> @@ -1,6 +1,5 @@
>>> // SPDX-License-Identifier: GPL-2.0-or-later
>>>
>>> -#include <linux/console.h>
>>> #include <linux/pci.h>
>>>
>>> #include <drm/drm_aperture.h>
>>> diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
>>> index 4706c5bc3067..659208d5aef9 100644
>>> --- a/drivers/gpu/drm/tiny/cirrus.c
>>> +++ b/drivers/gpu/drm/tiny/cirrus.c
>>> @@ -16,7 +16,6 @@
>>> * Copyright 1999-2001 Jeff Garzik <jgarzik@...ox.com>
>>> */
>>>
>>> -#include <linux/console.h>
>>> #include <linux/dma-buf-map.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
>>> index e4377c37cf33..b1e63fd543bb 100644
>>> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
>>> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
>>> @@ -7,7 +7,6 @@
>>> * Michael Thayer <michael.thayer@...cle.com,
>>> * Hans de Goede <hdegoede@...hat.com>
>>> */
>>> -#include <linux/console.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> #include <linux/vt_kern.h>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
>>> index 28200dfba2d1..ba9c0c2f8ae6 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
>>> @@ -27,7 +27,6 @@
>>> */
>>>
>>> #include <linux/module.h>
>>> -#include <linux/console.h>
>>> #include <linux/pci.h>
>>> #include <linux/poll.h>
>>> #include <linux/wait.h>
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>> index 05e9949293d5..115ec9518277 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>> @@ -25,7 +25,6 @@
>>> *
>>> **************************************************************************/
>>>
>>> -#include <linux/console.h>
>>> #include <linux/dma-mapping.h>
>>> #include <linux/module.h>
>>> #include <linux/pci.h>
>>> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
>>> index ef9c57ce0906..d4320b147956 100644
>>> --- a/drivers/video/console/vgacon.c
>>> +++ b/drivers/video/console/vgacon.c
>>> @@ -97,30 +97,9 @@ static int vga_video_font_height;
>>> static int vga_scan_lines __read_mostly;
>>> static unsigned int vga_rolled_over; /* last vc_origin offset before wrap */
>>>
>>> -static bool vgacon_text_mode_force;
>>> static bool vga_hardscroll_enabled;
>>> static bool vga_hardscroll_user_enable = true;
>>>
>>> -bool vgacon_text_force(void)
>>> -{
>>> - return vgacon_text_mode_force;
>>> -}
>>> -EXPORT_SYMBOL(vgacon_text_force);
>>> -
>>> -static int __init text_mode(char *str)
>>> -{
>>> - vgacon_text_mode_force = 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");
>>> - pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
>>> -
>>> - return 1;
>>> -}
>>> -
>>> -/* force text mode - used by kernel modesetting */
>>> -__setup("nomodeset", text_mode);
>>> -
>>> static int __init no_scroll(char *str)
>>> {
>>> /*
>>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>>> index 48b7de80daf5..18982d3507e4 100644
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -969,4 +969,10 @@ static inline int drm_mode_config_init(struct drm_device *dev)
>>> void drm_mode_config_reset(struct drm_device *dev);
>>> void drm_mode_config_cleanup(struct drm_device *dev);
>>>
>>> +#ifdef CONFIG_VGA_CONSOLE
>>> +extern int drm_check_modeset(void);
>>> +#else
>>> +static inline int drm_check_modeset(void) { return 0; }
>>> +#endif
>>> +
>>> #endif
>>> diff --git a/include/linux/console.h b/include/linux/console.h
>>> index 20874db50bc8..d4dd8384898b 100644
>>> --- a/include/linux/console.h
>>> +++ b/include/linux/console.h
>>> @@ -217,12 +217,6 @@ extern atomic_t ignore_console_lock_warning;
>>> #define VESA_HSYNC_SUSPEND 2
>>> #define VESA_POWERDOWN 3
>>>
>>> -#ifdef CONFIG_VGA_CONSOLE
>>> -extern bool vgacon_text_force(void);
>>> -#else
>>> -static inline bool vgacon_text_force(void) { return false; }
>>> -#endif
>>> -
>>> extern void console_init(void);
>>>
>>> /* For deferred console takeover */
>>>
>
--
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