| 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: <aVK28mBT6PwD7Rkr@fedora>
Date: Mon, 29 Dec 2025 18:14:26 +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 v3 33/33] drm/vkms: Introduce configfs for dynamic
connector creation
On Mon, Dec 22, 2025 at 11:11:35AM +0100, Louis Chauvet wrote:
> DRM allows the connector to be created after the device. To allows
> emulating this, add two configfs attributes to connector to allows this.
>
> Using the dynamic attribute you can set if a connector will be dynamic or
> not.
> Using the enabled attribute, you can set at runtime if a dynamic connector
> is present or not.
>
> Co-developed-by: José Expósito <jose.exposito89@...il.com>
> Signed-off-by: José Expósito <jose.exposito89@...il.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
> Documentation/ABI/testing/configfs-vkms | 14 +++
> Documentation/gpu/vkms.rst | 6 +-
> drivers/gpu/drm/vkms/vkms_configfs.c | 146 ++++++++++++++++++++++++++++++--
> 3 files changed, 155 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-vkms b/Documentation/ABI/testing/configfs-vkms
> index 4061ada5d88b..a7fce35fcf91 100644
> --- a/Documentation/ABI/testing/configfs-vkms
> +++ b/Documentation/ABI/testing/configfs-vkms
> @@ -62,6 +62,20 @@ Description:
> Content of the EDID for this connector. Ignored if
> edid_enabled is not set.
>
> +What: /sys/kernel/config/vkms/<device>/connectors/<connector>/dynamic
> +Date: Nov 2025
> +Contact: dri-devel@...ts.freedesktop.org
> +Description:
> + Set to 1 to create a dynamic connector (emulates DP MST).
> + Value: 1 - dynamic, 0 - static.
> +
> +What: /sys/kernel/config/vkms/<device>/connectors/<connector>/enabled
> +Date: Nov 2025
> +Contact: dri-devel@...ts.freedesktop.org
> +Description:
> + For dynamic connectors, set to 1 to create the connector,
> + 0 to remove it. Value: 1 - enabled, 0 - disabled.
> +
> What: /sys/kernel/config/vkms/<device>/connectors/<connector>/possible_encoders
> Date: Nov 2025
> Contact: dri-devel@...ts.freedesktop.org
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index 60367fd1bd65..fce229fbfc7c 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -138,7 +138,7 @@ Last but not least, create one or more connectors::
>
> sudo mkdir /config/vkms/my-vkms/connectors/connector0
>
> -Connectors have 5 configurable attribute:
> +Connectors have 7 configurable attribute:
>
> - status: Connection status: 1 connected, 2 disconnected, 3 unknown (same values
> as those exposed by the "status" property of a connector)
> @@ -150,7 +150,9 @@ Connectors have 5 configurable attribute:
> - edid_enabled: Enable or not EDID for this connector. Some connectors may not have an
> EDID but just a list of modes, this attribute allows to disable EDID property.
> - edid: Content of the EDID. Ignored if edid_enabled is not set
> -
> +- dynamic: Set to 1 while configuring the device to create a dynamic connector. A dynamic
> + connector can be used to emulate DP MST connectors.
> +- enabled: For dynamic connector, set it to 1 to create the connector, 0 to remove it.
>
> To finish the configuration, link the different pipeline items::
>
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index 20f5150e8b24..657381a8a4c2 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1144,6 +1144,12 @@ static ssize_t connector_status_show(struct config_item *item, char *page)
> return sprintf(page, "%u", status);
> }
>
> +static bool connector_is_enabled(struct vkms_config_connector *connector_cfg)
> +{
> + return !connector_cfg->dynamic ||
> + (connector_cfg->dynamic && connector_cfg->enabled);
> +}
> +
> static ssize_t connector_status_store(struct config_item *item,
> const char *page, size_t count)
> {
> @@ -1163,7 +1169,7 @@ static ssize_t connector_status_store(struct config_item *item,
> scoped_guard(mutex, &connector->dev->lock) {
> vkms_config_connector_set_status(connector->config, status);
>
> - if (connector->dev->enabled)
> + if (connector->dev->enabled && connector_is_enabled(connector->config))
> vkms_trigger_connector_hotplug(connector->dev->config->dev);
> }
>
> @@ -1224,7 +1230,7 @@ static ssize_t connector_type_store(struct config_item *item,
> }
>
> scoped_guard(mutex, &connector->dev->lock) {
> - if (connector->dev->enabled)
> + if (connector->dev->enabled && connector_is_enabled(connector->config))
> return -EBUSY;
>
> vkms_config_connector_set_type(connector->config, val);
> @@ -1343,6 +1349,107 @@ static ssize_t connector_edid_store(struct config_item *item,
> connector_status_disconnected)
> vkms_trigger_connector_hotplug(connector->dev->config->dev);
> }
> + return count;
> +}
> +
> +static ssize_t connector_enabled_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_connector *connector;
> + bool enabled;
> +
> + connector = connector_item_to_vkms_configfs_connector(item);
> +
> + scoped_guard(mutex, &connector->dev->lock)
> + enabled = vkms_config_connector_is_enabled(connector->config);
> +
> + return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t connector_enabled_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + struct vkms_configfs_connector *connector;
> + struct vkms_config_connector *connector_cfg;
> + bool enabled, was_enabled;
> +
> + connector = connector_item_to_vkms_configfs_connector(item);
> + connector_cfg = connector->config;
> +
> + if (kstrtobool(page, &enabled))
> + return -EINVAL;
> + scoped_guard(mutex, &connector->dev->lock) {
> + if (!connector->dev->enabled) {
> + vkms_config_connector_set_enabled(connector_cfg, enabled);
> + } else {
> + // Only dynamic connector can be enabled/disabled at runtime
> + if (!connector_cfg->dynamic)
> + return -EBUSY;
> +
> + was_enabled = vkms_config_connector_is_enabled(connector_cfg);
> + vkms_config_connector_set_enabled(connector_cfg, enabled);
> +
> + // Resulting configuration is invalid (missing encoder for example)
> + // Early return to avoid drm core issue
> + if (!vkms_config_is_valid(connector->dev->config)) {
> + count = -EINVAL;
> + goto rollback;
Since this rollback jumps out of the guard, there is a chance that, while the connector is
"enabled" (enabled as in the variable) and before is set to "was_enabled", another thread
is executed and reads an invalid value.
The cleanup path needs to be inside the guard.
Jose
> + }
> +
> + if (!was_enabled && enabled) {
> + // Adding the connector
> + connector_cfg->connector = vkms_connector_hot_add(connector->dev->config->dev,
> + connector_cfg);
> + if (IS_ERR(connector_cfg->connector)) {
> + count = PTR_ERR(connector_cfg->connector);
> + goto rollback;
> + }
> + } else if (was_enabled && !enabled) {
> + vkms_connector_hot_remove(connector->dev->config->dev,
> + connector_cfg->connector);
> + }
> + }
> + }
> + return count;
> +
> +rollback:
> + vkms_config_connector_set_enabled(connector_cfg, was_enabled);
> + return count;
> +}
> +
> +static ssize_t connector_dynamic_show(struct config_item *item, char *page)
> +{
> + struct vkms_configfs_connector *connector;
> + bool enabled;
> +
> + connector = connector_item_to_vkms_configfs_connector(item);
> +
> + scoped_guard(mutex, &connector->dev->lock) {
> + enabled = vkms_config_connector_is_dynamic(connector->config);
> + }
> +
> + return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t connector_dynamic_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + struct vkms_configfs_connector *connector;
> + struct vkms_config_connector *connector_cfg;
> + bool dynamic;
> +
> + connector = connector_item_to_vkms_configfs_connector(item);
> + connector_cfg = connector->config;
> +
> + if (kstrtobool(page, &dynamic))
> + return -EINVAL;
> +
> + scoped_guard(mutex, &connector->dev->lock) {
> + // Can't change the dynamic status when the device is activated
> + if (connector->dev->enabled)
> + return -EBUSY;
> +
> + vkms_config_connector_set_dynamic(connector_cfg, dynamic);
> + }
>
> return count;
> }
> @@ -1352,6 +1459,8 @@ CONFIGFS_ATTR(connector_, type);
> CONFIGFS_ATTR(connector_, supported_colorspaces);
> CONFIGFS_ATTR(connector_, edid_enabled);
> CONFIGFS_ATTR(connector_, edid);
> +CONFIGFS_ATTR(connector_, dynamic);
> +CONFIGFS_ATTR(connector_, enabled);
>
> static struct configfs_attribute *connector_item_attrs[] = {
> &connector_attr_status,
> @@ -1359,19 +1468,28 @@ static struct configfs_attribute *connector_item_attrs[] = {
> &connector_attr_supported_colorspaces,
> &connector_attr_edid_enabled,
> &connector_attr_edid,
> + &connector_attr_dynamic,
> + &connector_attr_enabled,
> NULL,
> };
>
> static void connector_release(struct config_item *item)
> {
> struct vkms_configfs_connector *connector;
> + struct vkms_config_connector *connector_cfg;
> struct mutex *lock;
>
> connector = connector_item_to_vkms_configfs_connector(item);
> + connector_cfg = connector->config;
> lock = &connector->dev->lock;
>
> scoped_guard(mutex, lock) {
> + if (connector->dev->enabled && connector_cfg->dynamic && connector_cfg->enabled)
> + vkms_connector_hot_remove(connector->dev->config->dev,
> + connector_cfg->connector);
> +
> vkms_config_destroy_connector(connector->config);
> +
> kfree(connector);
> }
> }
> @@ -1390,6 +1508,7 @@ static int connector_possible_encoders_allow_link(struct config_item *src,
> struct config_item *target)
> {
> struct vkms_configfs_connector *connector;
> + struct vkms_config_connector *connector_cfg;
> struct vkms_configfs_encoder *encoder;
> int ret;
>
> @@ -1397,16 +1516,26 @@ static int connector_possible_encoders_allow_link(struct config_item *src,
> return -EINVAL;
>
> connector = connector_possible_encoders_item_to_vkms_configfs_connector(src);
> + connector_cfg = connector->config;
> encoder = encoder_item_to_vkms_configfs_encoder(target);
>
> scoped_guard(mutex, &connector->dev->lock) {
> - if (connector->dev->enabled)
> - return -EBUSY;
> + if (connector->dev->enabled && connector_cfg->enabled) {
> + if (!connector_cfg->dynamic)
> + return -EBUSY;
> +
> + ret = vkms_connector_hot_attach_encoder(connector->dev->config->dev,
> + connector->config->connector,
> + encoder->config->encoder);
> + if (ret)
> + return ret;
> + }
>
> ret = vkms_config_connector_attach_encoder(connector->config,
> encoder->config);
> + if (ret)
> + return ret;
> }
> -
> return ret;
> }
>
> @@ -1445,9 +1574,6 @@ static struct config_group *make_connector_group(struct config_group *group,
> dev = child_group_to_vkms_configfs_device(group);
>
> scoped_guard(mutex, &dev->lock) {
> - if (dev->enabled)
> - return ERR_PTR(-EBUSY);
> -
> connector = kzalloc(sizeof(*connector), GFP_KERNEL);
> if (!connector)
> return ERR_PTR(-ENOMEM);
> @@ -1461,9 +1587,11 @@ static struct config_group *make_connector_group(struct config_group *group,
> return ERR_PTR(ret);
> }
>
> + vkms_config_connector_set_dynamic(connector->config, connector->dev->enabled);
> + vkms_config_connector_set_enabled(connector->config, !connector->dev->enabled);
> +
> config_group_init_type_name(&connector->group, name,
> &connector_item_type);
> -
> config_group_init_type_name(&connector->possible_encoders_group,
> "possible_encoders",
> &connector_possible_encoders_group_type);
>
> --
> 2.51.2
>
Powered by blists - more mailing lists