| 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: <aP85JP02YJdVkYJ2@fedora>
Date: Mon, 27 Oct 2025 10:19:32 +0100
From: José Expósito <jose.exposito89@...il.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: Haneen Mohammed <hamohammed.sa@...il.com>,
Simona Vetter <simona@...ll.ch>,
Melissa Wen <melissa.srw@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Jonathan Corbet <corbet@....net>,
victoria@...tem76.com, sebastian.wick@...hat.com,
thomas.petazzoni@...tlin.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 13/22] drm/vkms: Introduce configfs for plane zpos
property
On Sat, Oct 18, 2025 at 04:01:13AM +0200, Louis Chauvet wrote:
> Modern compositor rely on zpos managment to offload some processing to
> deticated hardware. In order to test multiple configurations, add zpos
> configuration to configFS.
>
> Introduce multiple attributes to configure zpos:
> - zpos_enabled - Create or not the zpos property. If not created, the zpos
> is undefined.
> - zpos_mutable - If the zpos property is created, allow or not the
> userspace to modify it
> - zpos_initial - Intial value for zpos property. Must be between zpos_min
> and zpos_max
> - zpos_min - Minimal zpos value for this plane. Must be smaller than or
> equals to zpos_max
> - zpos_max - Maximal zpos value for this plane. Must be greater than or
> equals to zpos_min
>
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
> Documentation/gpu/vkms.rst | 9 +-
> drivers/gpu/drm/vkms/vkms_configfs.c | 199 +++++++++++++++++++++++++++++++++++
> 2 files changed, 207 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index deb14e7c48ea..d4ad4af45414 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -87,7 +87,7 @@ Start by creating one or more planes::
>
> sudo mkdir /config/vkms/my-vkms/planes/plane0
>
> -Planes have 8 configurable attribute:
> +Planes have 13 configurable attribute:
>
> - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those
> exposed by the "type" property of a plane)
> @@ -111,6 +111,13 @@ Planes have 8 configurable attribute:
> To remove a format, use a minus and its fourcc: -XR24
> To add all formats use +*
> To remove all formats, use -*
> +- zpos_enabled: Enable or not the zpos property: 1 enable, 0 disable
> +- zpos_mutable: Create the zpos property as a mutable or imutable property: 1 mutable,
> + 0 disable. No effect if zpos_enabled is not set.
> +- zpos_initial: Set the initial zpos value. Must be between zpos_min and zpos_max. No
> + effect if zpos_enabled is not set.
> +- zpos_min: Set the minimal zpos value. No effect if zpos_enabled is not set.
> +- zpos_max: Set the maximal zpos value. No effect if zpos_enabled is not set.
Aren't zpos_min/max also ignored when zpos_mutable is false?
> Continue by creating one or more CRTCs::
>
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index 528f22fa2df1..fd1be7292058 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1,4 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0+
> +#include "asm-generic/errno-base.h"
Accidentally included by IDE?
> #include <linux/cleanup.h>
> #include <linux/configfs.h>
> #include <linux/mutex.h>
> @@ -679,6 +680,194 @@ static ssize_t plane_supported_formats_store(struct config_item *item,
> return count;
> }
>
> +static ssize_t plane_zpos_enabled_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_plane *plane;
> + bool enabled;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + scoped_guard(mutex, &plane->dev->lock)
> + enabled = vkms_config_plane_get_zpos_enabled(plane->config);
> +
> + return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t plane_zpos_enabled_store(struct config_item *item, const char *page,
> + size_t count)
> +{
> + struct vkms_configfs_plane *plane;
> + bool enabled;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + if (kstrtobool(page, &enabled))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &plane->dev->lock) {
> + if (plane->dev->enabled)
> + return -EBUSY;
> +
> + vkms_config_plane_set_zpos_enabled(plane->config, enabled);
> + }
> +
> + return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_mutable_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_plane *plane;
> + bool mutable;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + scoped_guard(mutex, &plane->dev->lock)
> + mutable = vkms_config_plane_get_zpos_mutable(plane->config);
> +
> + return sprintf(page, "%d\n", mutable);
> +}
> +
> +static ssize_t plane_zpos_mutable_store(struct config_item *item, const char *page,
> + size_t count)
> +{
> + struct vkms_configfs_plane *plane;
> + bool mutable;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + if (kstrtobool(page, &mutable))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &plane->dev->lock) {
> + if (plane->dev->enabled)
> + return -EBUSY;
> +
> + vkms_config_plane_set_zpos_mutable(plane->config, mutable);
> + }
> +
> + return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_initial_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int initial;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + scoped_guard(mutex, &plane->dev->lock)
> + initial = vkms_config_plane_get_zpos_initial(plane->config);
> +
> + return sprintf(page, "%u\n", initial);
> +}
> +
> +static ssize_t plane_zpos_initial_store(struct config_item *item, const char *page,
> + size_t count)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int initial;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + if (kstrtouint(page, 10, &initial))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &plane->dev->lock) {
> + if (plane->dev->enabled)
> + return -EBUSY;
> +
> + if (initial > vkms_config_plane_get_zpos_max(plane->config))
> + return -EINVAL;
> +
> + if (initial < vkms_config_plane_get_zpos_min(plane->config))
> + return -EINVAL;
> +
> + vkms_config_plane_set_zpos_initial(plane->config, initial);
> + }
> +
> + return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_min_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int min;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + scoped_guard(mutex, &plane->dev->lock)
> + min = vkms_config_plane_get_zpos_min(plane->config);
> +
> + return sprintf(page, "%u\n", min);
> +}
> +
> +static ssize_t plane_zpos_min_store(struct config_item *item, const char *page,
> + size_t count)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int min;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + if (kstrtouint(page, 10, &min))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &plane->dev->lock) {
> + if (plane->dev->enabled)
> + return -EBUSY;
> +
> + if (min > vkms_config_plane_get_zpos_max(plane->config))
> + return -EINVAL;
> +
> + if (min > vkms_config_plane_get_zpos_initial(plane->config))
> + return -EINVAL;
> +
> + vkms_config_plane_set_zpos_min(plane->config, min);
> + }
> +
> + return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_max_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int max;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + scoped_guard(mutex, &plane->dev->lock)
> + max = vkms_config_plane_get_zpos_max(plane->config);
> +
> + return sprintf(page, "%u\n", max);
> +}
> +
> +static ssize_t plane_zpos_max_store(struct config_item *item, const char *page,
> + size_t count)
> +{
> + struct vkms_configfs_plane *plane;
> + unsigned int max;
> +
> + plane = plane_item_to_vkms_configfs_plane(item);
> +
> + if (kstrtouint(page, 10, &max))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &plane->dev->lock) {
> + if (plane->dev->enabled)
> + return -EBUSY;
> +
> + if (max < vkms_config_plane_get_zpos_min(plane->config))
> + return -EINVAL;
> +
> + if (max < vkms_config_plane_get_zpos_initial(plane->config))
> + return -EINVAL;
> +
> + vkms_config_plane_set_zpos_max(plane->config, max);
> + }
> +
> + return (ssize_t)count;
> +}
> +
> CONFIGFS_ATTR(plane_, type);
> CONFIGFS_ATTR(plane_, supported_rotations);
> CONFIGFS_ATTR(plane_, default_rotation);
> @@ -687,6 +876,11 @@ CONFIGFS_ATTR(plane_, default_color_range);
> CONFIGFS_ATTR(plane_, supported_color_encoding);
> CONFIGFS_ATTR(plane_, default_color_encoding);
> CONFIGFS_ATTR(plane_, supported_formats);
> +CONFIGFS_ATTR(plane_, zpos_enabled);
> +CONFIGFS_ATTR(plane_, zpos_mutable);
> +CONFIGFS_ATTR(plane_, zpos_initial);
> +CONFIGFS_ATTR(plane_, zpos_min);
> +CONFIGFS_ATTR(plane_, zpos_max);
>
> static struct configfs_attribute *plane_item_attrs[] = {
> &plane_attr_type,
> @@ -697,6 +891,11 @@ static struct configfs_attribute *plane_item_attrs[] = {
> &plane_attr_supported_color_encoding,
> &plane_attr_default_color_encoding,
> &plane_attr_supported_formats,
> + &plane_attr_zpos_enabled,
> + &plane_attr_zpos_mutable,
> + &plane_attr_zpos_initial,
> + &plane_attr_zpos_min,
> + &plane_attr_zpos_max,
LGTM, other than my previous comment on adding validation in this layer.
It can force to change attributtes in a specific order. I think we should
only validate in vkms_config_is_valid().
> NULL,
> };
>
>
> --
> 2.51.0
>
Powered by blists - more mailing lists