linux-kernel - Re: [PATCH 00/13] drm/vkms: Allow to configure 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: <Z5uDGr445jEfdt5L@louis-chauvet-laptop>
Date: Thu, 30 Jan 2025 14:48:10 +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/13] drm/vkms: Allow to configure device

On 29/01/25 - 12:00, José Expósito wrote:
> Hi everyone,
> 
> In preparation for ConfigFS support, a flexible way to configure VKMS device(s)
> is required.
> This series adds the required APIs to create a configuration, the code changes
> required to apply it and KUnit test validating the changes.

Hi José,

Thanks a lot!

This series is amazing and better than mine on many points. I have few 
comments:
- a "strange" naming pair: add/destroy (I expect add/remove or 
  create/destroy like other function in DRM)
- usage of "complex" list accessors, can't we just create iterators?
- should we use pr_err in vkms_config_valid?

> Louis Chauvet and I are working on ConfigFS support. In this series I tried to
> merge his changes [1] with mine [2].
> I kept his Signed-off-by to reflect that, even if I show up as the author of
> some/most of the patches, this was a joint effort.

To avoid confusion, you should add the Co-developped-by tag, so it will be 
clear that we worked together on this.
 
> I'm still polishing the ConfigFS code [3] and its IGT tests [4] (connector
> hot-add/remove bugs) but the IGT tests also exercise this series and can be used
> for additional test coverage.

I will take a look at those series. For the connector hot-add/remove, do 
you have any example of usage in the kernel? I did not found anything in 
the documentation explaining they are hot-addable.

Thanks again for this series,
Louis Chauvet

> Best wishes,
> José Expósito
> 
> [1] https://patchwork.kernel.org/project/dri-devel/cover/20250121-google-remove-crtc-index-from-parameter-v3-0-cac00a3c3544@bootlin.com/
> [2] https://patchwork.kernel.org/project/dri-devel/cover/20240813105134.17439-1-jose.exposito89@gmail.com/
> [3] https://github.com/JoseExposito/linux/commits/patch-vkms-configfs/
> [4] https://gitlab.freedesktop.org/jexposit/igt-gpu-tools/-/commits/vkms-configfs
> 
> José Expósito (12):
>   drm/vkms: Extract vkms_connector header
>   drm/vkms: Add KUnit test scaffolding
>   drm/vkms: Extract vkms_config header
>   drm/vkms: Move default_config creation to its own function
>   drm/vkms: Set device name from vkms_config
>   drm/vkms: Allow to configure multiple planes
>   drm/vkms: Allow to configure multiple CRTCs
>   drm/vkms: Allow to attach planes and CRTCs
>   drm/vkms: Allow to configure multiple encoders
>   drm/vkms: Allow to attach encoders and CRTCs
>   drm/vkms: Allow to configure multiple connectors
>   drm/vkms: Allow to attach connectors and encoders
> 
> Louis Chauvet (1):
>   drm/vkms: Add a validation function for VKMS configuration
> 
>  drivers/gpu/drm/vkms/Kconfig                  |  15 +
>  drivers/gpu/drm/vkms/Makefile                 |   5 +-
>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>  drivers/gpu/drm/vkms/tests/Makefile           |   3 +
>  drivers/gpu/drm/vkms/tests/vkms_config_test.c | 782 +++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_config.c            | 784 ++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_config.h            | 479 +++++++++++
>  drivers/gpu/drm/vkms/vkms_connector.c         |  61 ++
>  drivers/gpu/drm/vkms/vkms_connector.h         |  26 +
>  drivers/gpu/drm/vkms/vkms_drv.c               |  45 +-
>  drivers/gpu/drm/vkms/vkms_drv.h               |  17 +-
>  drivers/gpu/drm/vkms/vkms_output.c            | 255 ++++--
>  12 files changed, 2337 insertions(+), 139 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/vkms/tests/Makefile
>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_config_test.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_config.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_config.h
>  create mode 100644 drivers/gpu/drm/vkms/vkms_connector.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_connector.h
> 
> -- 
> 2.48.1
>