| 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: <6620ac6e-ffe4-f7b9-2213-1a4e3382cf9a@chromium.org>
Date: Fri, 18 Aug 2023 14:39:08 +0900
From: Brandon Ross Pollack <brpol@...omium.org>
To: Maira Canal <mairacanal@...eup.net>,
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
On 6/26/23 03:04, Maira Canal wrote:
> 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"?
At the risk of being annoyingly pedantic, I actually like the original
name better because it makes it clear it is an entire device and setup,
including the planes/encoders/connectors/crtcs needed and connecting
them as necessary etc. I just feel "device" here makes it clearer what
is the default thing.
>
> Also, could you add this parameter to vkms_config debugfs file?
But of course! This is the last comment before I send out the new series.
Thank you so much for your time, looking forward to the next round of
comments.
>
> 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);
>> }
>
> From mboxrd@z Thu Jan 1 00:00:00 1970
> Return-Path: <dri-devel-bounces@...ts.freedesktop.org>
> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> aws-us-west-2-korg-lkml-1.web.codeaurora.org
> Received: from gabe.freedesktop.org (gabe.freedesktop.org
> [131.252.210.177])
> (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
> bits))
> (No client certificate requested)
> by smtp.lore.kernel.org (Postfix) with ESMTPS id AB561C0015E
> for <dri-devel@...hiver.kernel.org>; Sun, 25 Jun 2023 18:04:15
> +0000 (UTC)
> Received: from gabe.freedesktop.org (localhost [127.0.0.1])
> by gabe.freedesktop.org (Postfix) with ESMTP id D153310E18C;
> Sun, 25 Jun 2023 18:04:14 +0000 (UTC)
> Received: from mx1.riseup.net (mx1.riseup.net [198.252.153.129])
> by gabe.freedesktop.org (Postfix) with ESMTPS id C6BE910E187
> for <dri-devel@...ts.freedesktop.org>; Sun, 25 Jun 2023 18:04:12 +0000
> (UTC)
> Received: from fews01-sea.riseup.net (fews01-sea-pn.riseup.net
> [10.0.1.109])
> (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
> key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest
> SHA256)
> (No client certificate requested)
> by mx1.riseup.net (Postfix) with ESMTPS id 4QpzPl6CHTzDrNy;
> Sun, 25 Jun 2023 18:04:11 +0000 (UTC)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=riseup.net;
> s=squak;
> t=1687716252; bh=3cOd9Tulf+RLZ2R/lGuVfEdERy8xqZ4HHWjkWZv3FAs=;
> h=Date:Subject:To:Cc:References:From:In-Reply-To:From;
> b=lXentHTOo29YwgA/Q5WAgBBEUsl5DTqsmOJd4wsjxBgZocgZ/PG9hmyBm6a2IjvcQ
> jSPI26UWubdYOe7kngWhcU0/oO570ofVUyrxiNgiJQfcscdp5bpwrskBKK4yXdJc0q
> 2YM4+n0xeBBSr2pFLMDwbOsLDvaGUK77nSPcbPXQ=
> X-Riseup-User-ID:
> 7EBE90DED2258EE1CA830B796CBA05FD63338D890F31F4C1BCCCB27A6DA77A56
> Received: from [127.0.0.1] (localhost [127.0.0.1])
> by fews01-sea.riseup.net (Postfix) with ESMTPSA id 4QpzPg4YpbzJntB;
> Sun, 25 Jun 2023 18:04:07 +0000 (UTC)
> Message-ID: <64c40359-d0ee-5070-2a52-033c7e655e0a@...eup.net>
> Date: Sun, 25 Jun 2023 15:04:05 -0300
> MIME-Version: 1.0
> Subject: Re: [PATCH v2 6/6] drm/vkms: Add a module param to
> enable/disable the
> default device
> Content-Language: en-US
> 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>
> References: <20230623222353.97283-1-jshargo@...omium.org>
> <20230623222353.97283-7-jshargo@...omium.org>
> From: Maira Canal <mairacanal@...eup.net>
> In-Reply-To: <20230623222353.97283-7-jshargo@...omium.org>
> Content-Type: text/plain; charset=UTF-8; format=flowed
> Content-Transfer-Encoding: 8bit
> X-BeenThere: dri-devel@...ts.freedesktop.org
> X-Mailman-Version: 2.1.29
> Precedence: list
> List-Id: Direct Rendering Infrastructure - Development
> <dri-devel.lists.freedesktop.org>
> List-Unsubscribe:
> <https://lists.freedesktop.org/mailman/options/dri-devel>,
> <mailto:dri-devel-request@...ts.freedesktop.org?subject=unsubscribe>
> List-Archive: <https://lists.freedesktop.org/archives/dri-devel>
> List-Post: <mailto:dri-devel@...ts.freedesktop.org>
> List-Help: <mailto:dri-devel-request@...ts.freedesktop.org?subject=help>
> List-Subscribe:
> <https://lists.freedesktop.org/mailman/listinfo/dri-devel>,
> <mailto:dri-devel-request@...ts.freedesktop.org?subject=subscribe>
> Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
> linux-doc@...r.kernel.org
> Errors-To: dri-devel-bounces@...ts.freedesktop.org
> Sender: "dri-devel" <dri-devel-bounces@...ts.freedesktop.org>
>
> 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);
>> }
>
Powered by blists - more mailing lists