From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5CFCF32142E for ; Mon, 29 Dec 2025 14:40:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767019233; cv=none; b=nAynqAMCqWrHU4OJZvPgEl+9WBrAVXNtDCDXdMZXZD0ZnF9SK1XjqJaM4xVJJNKjyNNTYg3l5IVuq13EPF8fyaFEzWoKpNywCxz0fwPv7ZyzuWOJXGEZhDAnHxvpWoS6FD40BPtfVces58srx5sXesF9XJxfM0XP3cIjwgyiiIA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767019233; c=relaxed/simple; bh=rOJwSwgN8Nyopi6nowMsKxMEN9cRq1r4JghdluU4gKk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=V1qN4NRk3cV7yRIbMqOCCUxqY0HXIkTFQqfmcxBQ2SmI0ekyAbRnSCNvTU4HJ+C+xXqMJTg5c3ZbXn5ytWjRDMXKFG1a8umMzwMbdf+xBSISQeTXpLlWNp6PHFDyI0AqkiL8E7hAh+Fi6mCoP8Dd0jZY0TYl9FrE0AkJSnvPaUc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=hmJJffzS; arc=none smtp.client-ip=185.246.85.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="hmJJffzS" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 4FB4D4E41E42; Mon, 29 Dec 2025 14:40:28 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 09F0860725; Mon, 29 Dec 2025 14:40:28 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 6EE8A113B061F; Mon, 29 Dec 2025 15:40:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1767019212; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=KHiNO49s1LI0Asee/eQKQNGC6G1Km0lH02lOMMvuT0Q=; b=hmJJffzSghd8i6EFJlxW1J4CjGkBxhcfMj0/Gyj2yTxtNeMYsLSuGZ/Y93uBGEtY3xuLmw zIW786/cVknjj6MsV3hz9hA4XI7cMbZx3TSJjRuR/lyejFIjK2HnJFKo9obUtaidLyQg4O vtPoQR8KpOMcvtp/fE3GAKMfkPiss5LTPY2swc3kA1hMAgDnRIOAz618GSNnib3aJekA7W 8RHs11JJNOGZ/6coexRSwLVkwllX4DD8ox2ofIubWeGVN3mgrQeNzIsFMJo+imI1WVIj1f XO5VVhhik65XyrRrzO8r4SJN8T6Iw262mFMRcPBDpAaMFMj2nTVaqEW9yHTm3Q== Message-ID: Date: Mon, 29 Dec 2025 15:40:23 +0100 Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 07/33] drm/vkms: Introduce configfs for plane name To: Luca Ceresoli , Haneen Mohammed , Simona Vetter , Melissa Wen , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , jose.exposito89@gmail.com, Jonathan Corbet Cc: victoria@system76.com, sebastian.wick@redhat.com, thomas.petazzoni@bootlin.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org References: <20251222-vkms-all-config-v3-0-ba42dc3fb9ff@bootlin.com> <20251222-vkms-all-config-v3-7-ba42dc3fb9ff@bootlin.com> From: Louis Chauvet Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 On 12/23/25 12:14, Luca Ceresoli wrote: > On Mon Dec 22, 2025 at 11:11 AM CET, Louis Chauvet wrote: >> Planes can have name, create a plane attribute to configure it. Currently >> plane name is mainly used in logs. >> >> Signed-off-by: Louis Chauvet >> --- >> Documentation/ABI/testing/configfs-vkms | 6 +++++ >> Documentation/gpu/vkms.rst | 3 ++- >> drivers/gpu/drm/vkms/vkms_configfs.c | 43 +++++++++++++++++++++++++++++++++ >> 3 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/ABI/testing/configfs-vkms b/Documentation/ABI/testing/configfs-vkms >> index 0beaa25f30ba..6fe375d1636f 100644 >> --- a/Documentation/ABI/testing/configfs-vkms >> +++ b/Documentation/ABI/testing/configfs-vkms >> @@ -103,6 +103,12 @@ Description: >> Plane type. Possible values: 0 - overlay, 1 - primary, >> 2 - cursor. >> >> +What: /sys/kernel/config/vkms//planes//name >> +Date: Nov 2025 >> +Contact: dri-devel@lists.freedesktop.org >> +Description: >> + Name of the plane. >> + >> What: /sys/kernel/config/vkms//planes//possible_crtcs >> Date: Nov 2025 >> Contact: dri-devel@lists.freedesktop.org >> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst >> index 1e79e62a6bc4..79f1185d8645 100644 >> --- a/Documentation/gpu/vkms.rst >> +++ b/Documentation/gpu/vkms.rst >> @@ -87,10 +87,11 @@ Start by creating one or more planes:: >> >> sudo mkdir /config/vkms/my-vkms/planes/plane0 >> >> -Planes have 1 configurable attribute: >> +Planes have 2 configurable attributes: >> >> - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those >> exposed by the "type" property of a plane) >> +- name: Name of the plane. Allowed characters are [A-Za-z1-9_-] >> >> 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 506666e21c91..989788042191 100644 >> --- a/drivers/gpu/drm/vkms/vkms_configfs.c >> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c >> @@ -324,10 +324,53 @@ static ssize_t plane_type_store(struct config_item *item, const char *page, >> return (ssize_t)count; >> } >> >> +static ssize_t plane_name_show(struct config_item *item, char *page) >> +{ >> + struct vkms_configfs_plane *plane; >> + const char *name; >> + >> + plane = plane_item_to_vkms_configfs_plane(item); >> + >> + scoped_guard(mutex, &plane->dev->lock) >> + name = vkms_config_plane_get_name(plane->config); > > vkms_config_plane_get_name() returns a pointer to the name string, not a > copy. Unless I'm missing something, that string might be freed before the > next lines, where it is used: > >> + >> + if (name) >> + return sprintf(page, "%s\n", name); >> + return sprintf(page, "\n"); > > So for safety the above 3 lines whould go inside the scoped_guard(). Good catch! This also raised some questions on the whole locking synchronization between configfs / config / DRM core. I will work on this topic and maybe move the mutex / add a refcount to vkms_config. >> +} >> + >> +static ssize_t plane_name_store(struct config_item *item, const char *page, >> + size_t count) >> +{ >> + struct vkms_configfs_plane *plane; >> + size_t str_len; >> + >> + plane = plane_item_to_vkms_configfs_plane(item); >> + >> + // strspn is not lenght-protected, ensure that page is a null-terminated string. >> + str_len = strnlen(page, count); >> + if (str_len >= count) >> + return -EINVAL; >> + >> + if (strspn(page, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-") != count - 1) >> + return -EINVAL; > > I see you effor to make this as clean as possible, thanks. Still this is a > tad ugly, and should be moved to some common place at some point IMO. For > now it's fine, but if you need to add more user-passed strings, that could > be the moment to move this code. There are multiple "user strings" in this file (notably group names), but currently without limitation. I can create a tiny helper and limit all user strings to a-zA-Z0-9_- It will technically break the ABI, but I don't think this is a big issue. Do you or José think this is a good idea? If so I can extract the helper for v4 and send a separate series to do the limitation on other strings. > Luca > > -- > Luca Ceresoli, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com