linux-kernel - Re: [PATCH 00/16] drm/vkms: Add configfs support

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: <Z72jJtFCMypHpW_K@louis-chauvet-laptop>
Date: Tue, 25 Feb 2025 12:01:58 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: José Expósito <jose.exposito89@...il.com>
Cc: hamohammed.sa@...il.com, simona@...ll.ch, melissa.srw@...il.com,
	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: [PATCH 00/16] drm/vkms: Add configfs support

On 18/02/25 - 18:07, José Expósito wrote:
> Hi everyone,
> 
> This series, to be applied on top of [1], allow to configure one or more VKMS
> instances without having to reload the driver using configfs.
> 
> The series is structured in 3 blocks:
> 
>   - Patches 1..11: Basic device configuration. For simplicity, I kept the
>     available options as minimal as possible.

Thanks for this series, it is really nice!

I did some review, that can be sumarized in two point:
- Do we want to use scoped_guard?
- -EPERM, -EINVAL or -EBUSY when attempting to do something while the
  device is enabled
 
>   - Patches 12, 13 and 14: Allow to hot-plug and unplug connectors. This is not
>     part of the minimal set of options, but I included in this series so it can
>     be used as a template/example of how new configurations can be added.
> 
>   - Patches 15 and 16: New option to skip the default device creation and to-do
>     cleanup.

For the next iteration, can you move those patch before 12, 13, 14, so
1..11 could be merged before 12..14 (I need to think about the connector 
API to check if it will works with dynamic connector)?
 
> The process of configuring a VKMS device is documented in "vkms.rst".

This is a really good documentation!

> Finally, the code is thoroughly tested by a collection of IGT tests [2].

I quickly looked on them, it seems nice! Thank you for this huge
improvment!

Louis Chauvet

> Best wishes,
> José Expósito
> 
> [1] https://lore.kernel.org/all/20250218101214.5790-1-jose.exposito89@gmail.com/
> [2] It is not in patchwork yet, but it'll appear here eventually:
>     https://patchwork.freedesktop.org/project/igt/patches/?submitter=19782&state=*&q=&archive=both&delegate=
> 
> José Expósito (16):
>   drm/vkms: Expose device creation and destruction
>   drm/vkms: Add and remove VKMS instances via configfs
>   drm/vkms: Allow to configure multiple planes via configfs
>   drm/vkms: Allow to configure the plane type via configfs
>   drm/vkms: Allow to configure multiple CRTCs via configfs
>   drm/vkms: Allow to configure CRTC writeback support via configfs
>   drm/vkms: Allow to attach planes and CRTCs via configfs
>   drm/vkms: Allow to configure multiple encoders via configfs
>   drm/vkms: Allow to attach encoders and CRTCs via configfs
>   drm/vkms: Allow to configure multiple connectors via configfs
>   drm/vkms: Allow to attach connectors and encoders via configfs
>   drm/vkms: Allow to configure connector status
>   drm/vkms: Allow to update the connector status
>   drm/vkms: Allow to configure connector status via configfs
>   drm/vkms: Allow to configure the default device creation
>   drm/vkms: Remove completed task from the TODO list
> 
>  Documentation/gpu/vkms.rst                    |  98 +-
>  drivers/gpu/drm/vkms/Kconfig                  |   1 +
>  drivers/gpu/drm/vkms/Makefile                 |   3 +-
>  drivers/gpu/drm/vkms/tests/vkms_config_test.c |  24 +
>  drivers/gpu/drm/vkms/vkms_config.c            |   8 +-
>  drivers/gpu/drm/vkms/vkms_config.h            |  26 +
>  drivers/gpu/drm/vkms/vkms_configfs.c          | 918 ++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_configfs.h          |   8 +
>  drivers/gpu/drm/vkms/vkms_connector.c         |  26 +-
>  drivers/gpu/drm/vkms/vkms_connector.h         |  18 +-
>  drivers/gpu/drm/vkms/vkms_drv.c               |  18 +-
>  drivers/gpu/drm/vkms/vkms_drv.h               |   4 +
>  drivers/gpu/drm/vkms/vkms_output.c            |   2 +-
>  13 files changed, 1138 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.h
> 
> 
> base-commit: 9b6c03cb96b9e19bce2c2764d2c6dd4ccbd06c5d
> prerequisite-patch-id: 1bff7bbc4ef0e29266265ac3dc009011c046f745
> prerequisite-patch-id: 74a284d40a426a0038a7054068192238f7658187
> prerequisite-patch-id: c3e34e88ad6a0acf7d9ded0cdb4745a87cf6fd82
> prerequisite-patch-id: 9cd0dfaf8e21a811edbe5a2da7185b6f9055d42d
> prerequisite-patch-id: f50c41578b639370a5d610af6f25c2077321a886
> prerequisite-patch-id: 5a7219a51e42de002b8dbf94ec8af96320043489
> prerequisite-patch-id: 67ea5d4e21b4ce4acbd6fc3ce83017f55811c49b
> prerequisite-patch-id: 37a7fab113a32581f053c09f45efb137afd75a1b
> prerequisite-patch-id: 475bcdc6267f4b02fb1bb2379145529c33684e4f
> prerequisite-patch-id: d3114f0b3da3d8b5ad64692df761f1cf42fbdf12
> prerequisite-patch-id: d1d9280fb056130df2050a09b7ea7e7ddde007c5
> prerequisite-patch-id: 2c370f3de6d227fa8881212207978cce7bbb18ba
> prerequisite-patch-id: 938b8fe5437e5f7bc22bffc55ae249a27d399d66
> prerequisite-patch-id: ab0a510994fbe9985dc46a3d35e6d0574ddbb633
> -- 
> 2.48.1
>