linux-kernel - Re: [RFC PATCH 03/17] drm/vkms: Set device name from vkms

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: <Zrue381iv_jpPhnz@louis-chauvet-laptop>
Date: Tue, 13 Aug 2024 19:58:55 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: José Expósito <jose.exposito89@...il.com>
Cc: rodrigosiqueiramelo@...il.com, melissa.srw@...il.com,
	mairacanal@...eup.net, hamohammed.sa@...il.com, daniel@...ll.ch,
	maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
	tzimmermann@...e.de, airlied@...il.com,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 03/17] drm/vkms: Set device name from vkms_config

Le 13/08/24 - 12:44, José Expósito a écrit :
> In order to be able to create multiple devices, the device name needs to
> be unique.
> 
> Allow to set it in the VKMS configuration.
> 
> Signed-off-by: José Expósito <jose.exposito89@...il.com>
> ---
>  drivers/gpu/drm/vkms/vkms_config.c | 7 +++++--
>  drivers/gpu/drm/vkms/vkms_config.h | 3 ++-
>  drivers/gpu/drm/vkms/vkms_drv.c    | 2 +-
>  drivers/gpu/drm/vkms/vkms_drv.h    | 2 ++
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index 4c7cda16dfd8..6ab8091bf72f 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -8,7 +8,7 @@
>  #include "vkms_config.h"
>  #include "vkms_drv.h"
>  
> -struct vkms_config *vkms_config_create(void)
> +struct vkms_config *vkms_config_create(char *dev_name)
>  {
>  	struct vkms_config *config;
>  
> @@ -16,6 +16,8 @@ struct vkms_config *vkms_config_create(void)
>  	if (!config)
>  		return ERR_PTR(-ENOMEM);
>  
> +	config->dev_name = dev_name;
> +
>  	return config;
>  }

I think it can make sense to copy the device name here. If I look at your 
ConfigFS implementation, this pointer is managed by configfs, not vkms. 

I think it could be nice to avoid mixing different lifetime and let vkms 
fully manage its own objects.
  
> @@ -25,7 +27,7 @@ struct vkms_config *vkms_config_default_create(bool enable_cursor,
>  {
>  	struct vkms_config *config;
>  
> -	config = vkms_config_create();
> +	config = vkms_config_create(DEFAULT_DEVICE_NAME);
>  	if (IS_ERR(config))
>  		return config;
>  
> @@ -47,6 +49,7 @@ static int vkms_config_show(struct seq_file *m, void *data)
>  	struct drm_device *dev = entry->dev;
>  	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
>  
> +	seq_printf(m, "dev_name=%s\n", vkmsdev->config->dev_name);
>  	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
>  	seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
>  	seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 83181760b02b..ba06aad32799 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -8,6 +8,7 @@
>  struct vkms_device;
>  
>  struct vkms_config {
> +	char *dev_name;
>  	bool writeback;
>  	bool cursor;
>  	bool overlay;
> @@ -16,7 +17,7 @@ struct vkms_config {
>  };
>  
>  /* VKMS Config */
> -struct vkms_config *vkms_config_create(void);
> +struct vkms_config *vkms_config_create(char *dev_name);
>  struct vkms_config *vkms_config_default_create(bool enable_cursor,
>  					       bool enable_writeback,
>  					       bool enable_overlay);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 4e36989589f9..2f9d1db0cfae 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -160,7 +160,7 @@ static int vkms_create(struct vkms_config *config)
>  	struct platform_device *pdev;
>  	struct vkms_device *vkms_device;
>  
> -	pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
> +	pdev = platform_device_register_simple(config->dev_name, -1, NULL, 0);
>  	if (IS_ERR(pdev))
>  		return PTR_ERR(pdev);
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 5c523ca27f22..87e44b51a03f 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -12,6 +12,8 @@
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_writeback.h>
>  
> +#define DEFAULT_DEVICE_NAME "vkms"
> +
>  #define XRES_MIN    10
>  #define YRES_MIN    10
>  
> -- 
> 2.46.0
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com