linux-kernel - Re: [PATCH v2 6/6] drm/vkms: Add a module param to enable/disable the default device

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: <64c40359-d0ee-5070-2a52-033c7e655e0a@riseup.net>
Date:   Sun, 25 Jun 2023 15:04:05 -0300
From:   Maira Canal <mairacanal@...eup.net>
To:     Jim Shargo <jshargo@...omium.org>, Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...il.com>,
        Haneen Mohammed <hamohammed.sa@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Melissa Wen <melissa.srw@...il.com>,
        Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
        Thomas Zimmermann <tzimmermann@...e.de>
Cc:     dri-devel@...ts.freedesktop.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/6] drm/vkms: Add a module param to enable/disable the
 default device

Hi Jim,

On 6/23/23 19:23, Jim Shargo wrote:
> In many testing circumstances, we will want to just create a new device
> and test against that. If we create a default device, it can be annoying
> to have to manually select the new device instead of choosing the only
> one that exists.
> 
> The param, enable_default, is defaulted to true to maintain backwards
> compatibility.
> 
> Signed-off-by: Jim Shargo <jshargo@...omium.org>
> ---
>   drivers/gpu/drm/vkms/vkms_drv.c | 44 ++++++++++++++++++++++-----------
>   1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 314a04659c5f..1cb56c090a65 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -42,17 +42,26 @@
>   #define DRIVER_MAJOR	1
>   #define DRIVER_MINOR	0
>   
> +static bool enable_default_device = true;
> +module_param_named(enable_default_device, enable_default_device, bool, 0444);
> +MODULE_PARM_DESC(enable_default_device,
> +		 "Enable/Disable creating the default device");

Wouldn't be better to just call it "enable_default"?

Also, could you add this parameter to vkms_config debugfs file?

Best Regards,
- Maíra

> +
>   static bool enable_cursor = true;
>   module_param_named(enable_cursor, enable_cursor, bool, 0444);
> -MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
> +MODULE_PARM_DESC(enable_cursor,
> +		 "Enable/Disable cursor support for the default device");
>   
>   static bool enable_writeback = true;
>   module_param_named(enable_writeback, enable_writeback, bool, 0444);
> -MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector support");
> +MODULE_PARM_DESC(
> +	enable_writeback,
> +	"Enable/Disable writeback connector support for the default device");
>   
>   static bool enable_overlay;
>   module_param_named(enable_overlay, enable_overlay, bool, 0444);
> -MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
> +MODULE_PARM_DESC(enable_overlay,
> +		 "Enable/Disable overlay support for the default device");
>   
>   DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>   
> @@ -278,10 +287,7 @@ void vkms_remove_device(struct vkms_device *vkms_device)
>   static int __init vkms_init(void)
>   {
>   	int ret;
> -	struct platform_device *pdev;
> -	struct vkms_device_setup vkms_device_setup = {
> -		.configfs = NULL,
> -	};
> +	struct platform_device *default_pdev = NULL;
>   
>   	ret = platform_driver_register(&vkms_platform_driver);
>   	if (ret) {
> @@ -289,19 +295,27 @@ static int __init vkms_init(void)
>   		return ret;
>   	}
>   
> -	pdev = platform_device_register_data(NULL, DRIVER_NAME, 0,
> -					     &vkms_device_setup,
> -					     sizeof(vkms_device_setup));
> -	if (IS_ERR(pdev)) {
> -		DRM_ERROR("Unable to register default vkms device\n");
> -		platform_driver_unregister(&vkms_platform_driver);
> -		return PTR_ERR(pdev);
> +	if (enable_default_device) {
> +		struct vkms_device_setup vkms_device_setup = {
> +			.configfs = NULL,
> +		};
> +
> +		default_pdev = platform_device_register_data(
> +			NULL, DRIVER_NAME, 0, &vkms_device_setup,
> +			sizeof(vkms_device_setup));
> +		if (IS_ERR(default_pdev)) {
> +			DRM_ERROR("Unable to register default vkms device\n");
> +			platform_driver_unregister(&vkms_platform_driver);
> +			return PTR_ERR(default_pdev);
> +		}
>   	}
>   
>   	ret = vkms_init_configfs();
>   	if (ret) {
>   		DRM_ERROR("Unable to initialize configfs\n");
> -		platform_device_unregister(pdev);
> +		if (default_pdev)
> +			platform_device_unregister(default_pdev);
> +
>   		platform_driver_unregister(&vkms_platform_driver);
>   	}
>