Linux Documentation
 help / color / mirror / Atom feed
* [PATCH v4 11/13] ima: Support staging and deleting N measurements entries
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Add support for sending a value N between 1 and ULONG_MAX to the staging
interface. This value represents the number of measurements that should be
deleted from the current measurements list.

This staging method allows the remote attestation agents to easily separate
the measurements that were verified (staged and deleted) from those that
weren't due to the race between taking a TPM quote and reading the
measurements list.

In order to minimize the locking time of ima_extend_list_mutex, deleting
N entries is realized by staging the entire current measurements list
(with the lock), by determining the N-th staged entry (without the lock),
and by splicing the entries in excess back to the current measurements list
(with the lock). Finally, the N entries are deleted (without the lock).

Flushing the hash table is not supported for N entries, since it would
require removing the N entries one by one from the hash table under the
ima_extend_list_mutex lock, which would increase the locking time.

The ima_extend_list_mutex lock is necessary in ima_dump_measurement_list()
because ima_queue_staged_delete_partial() uses __list_cut_position() to
modify ima_measurements_staged, for which no RCU-safe variant exists. For
the staging with prompt flavor alone, list_replace_rcu() could have been
used instead, but since both flavors share the same kexec serialization
path, the mutex is required regardless.

Link: https://github.com/linux-integrity/linux/issues/1
Suggested-by: Steven Chen <chenste@linux.microsoft.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/Kconfig     |  3 ++
 security/integrity/ima/ima.h       |  1 +
 security/integrity/ima/ima_fs.c    | 22 +++++++++-
 security/integrity/ima/ima_queue.c | 70 ++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index e714726f3384..6ddb4e77bff5 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -341,6 +341,9 @@ config IMA_STAGING
 	  It allows user space to stage the measurements list for deletion and
 	  to delete the staged measurements after confirmation.
 
+	  Or, alternatively, it allows user space to specify N measurements
+	  entries to be deleted.
+
 	  On kexec, staging is reverted and staged measurements are prepended
 	  to the current measurements list when measurements are copied to the
 	  secondary kernel.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 699b735dec7d..de0693fce53c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -319,6 +319,7 @@ struct ima_template_desc *lookup_template_desc(const char *name);
 bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
 int ima_queue_stage(void);
 int ima_queue_staged_delete_all(void);
+int ima_queue_staged_delete_partial(unsigned long req_value);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 39d9128e9f22..eb3f343c1138 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -28,6 +28,7 @@
  * Requests:
  * 'A\n': stage the entire measurements list
  * 'D\n': delete all staged measurements
+ * '[1, ULONG_MAX]\n' delete N measurements entries
  */
 #define STAGED_REQ_LENGTH 21
 
@@ -319,6 +320,7 @@ static ssize_t ima_measurements_staged_write(struct file *file,
 					     size_t datalen, loff_t *ppos)
 {
 	char req[STAGED_REQ_LENGTH];
+	unsigned long req_value;
 	int ret;
 
 	if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
@@ -346,7 +348,25 @@ static ssize_t ima_measurements_staged_write(struct file *file,
 		ret = ima_queue_staged_delete_all();
 		break;
 	default:
-		ret = -EINVAL;
+		if (ima_flush_htable) {
+			pr_debug("Deleting staged N measurements not supported when flushing the hash table is requested\n");
+			return -EINVAL;
+		}
+
+		ret = kstrtoul(req, 10, &req_value);
+		if (ret < 0)
+			return ret;
+
+		if (req_value == 0) {
+			pr_debug("Must delete at least one entry\n");
+			return -EINVAL;
+		}
+
+		ret = ima_queue_stage();
+		if (ret < 0)
+			return ret;
+
+		ret = ima_queue_staged_delete_partial(req_value);
 	}
 
 	if (ret < 0)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index f5c18acfbc43..4fb557d61a88 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -371,6 +371,76 @@ int ima_queue_staged_delete_all(void)
 	return 0;
 }
 
+int ima_queue_staged_delete_partial(unsigned long req_value)
+{
+	unsigned long req_value_copy = req_value;
+	unsigned long size_to_remove = 0, num_to_remove = 0;
+	struct list_head *cut_pos = NULL;
+	LIST_HEAD(ima_measurements_trim);
+	struct ima_queue_entry *qe;
+	int ret = 0;
+
+	/*
+	 * Safe walk (no concurrent write), not under ima_extend_list_mutex
+	 * for performance reasons.
+	 */
+	list_for_each_entry(qe, &ima_measurements_staged, later) {
+		size_to_remove += get_binary_runtime_size(qe->entry);
+		num_to_remove++;
+
+		if (--req_value_copy == 0) {
+			/* qe->later always points to a valid list entry. */
+			cut_pos = &qe->later;
+			break;
+		}
+	}
+
+	/* Nothing to remove, undoing staging. */
+	if (req_value_copy > 0) {
+		size_to_remove = 0;
+		num_to_remove = 0;
+		ret = -ENOENT;
+	}
+
+	mutex_lock(&ima_extend_list_mutex);
+	if (list_empty(&ima_measurements_staged)) {
+		mutex_unlock(&ima_extend_list_mutex);
+		return -ENOENT;
+	}
+
+	if (cut_pos != NULL)
+		/*
+		 * ima_dump_measurement_list() does not modify the list,
+		 * cut_pos remains the same even if it was computed before
+		 * the lock.
+		 */
+		__list_cut_position(&ima_measurements_trim,
+				    &ima_measurements_staged, cut_pos);
+
+	atomic_long_sub(num_to_remove, &ima_num_entries[BINARY_STAGED]);
+	atomic_long_add(atomic_long_read(&ima_num_entries[BINARY_STAGED]),
+			&ima_num_entries[BINARY]);
+	atomic_long_set(&ima_num_entries[BINARY_STAGED], 0);
+
+	if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
+		binary_runtime_size[BINARY_STAGED] -= size_to_remove;
+		binary_runtime_size[BINARY] +=
+					binary_runtime_size[BINARY_STAGED];
+		binary_runtime_size[BINARY_STAGED] = 0;
+	}
+
+	/*
+	 * Splice (prepend) any remaining non-deleted staged entries to the
+	 * active list (RCU not needed, there cannot be concurrent readers).
+	 */
+	list_splice(&ima_measurements_staged, &ima_measurements);
+	INIT_LIST_HEAD(&ima_measurements_staged);
+	mutex_unlock(&ima_extend_list_mutex);
+
+	ima_queue_delete(&ima_measurements_trim, false);
+	return ret;
+}
+
 static void ima_queue_delete(struct list_head *head, bool flush_htable)
 {
 	struct ima_queue_entry *qe, *qe_tmp;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 10/13] ima: Add support for flushing the hash table when staging measurements
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce the new kernel option ima_flush_htable to decide whether or not
the digests of staged measurement entries are flushed from the hash table,
when they are deleted.

When the option is enabled, replace the old hash table with a new one,
by calling ima_alloc_replace_htable(), and completely delete the
measurements entries.

Note: This code derives from the Alt-IMA Huawei project, whose license is
      GPL-2.0 OR MIT.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  4 +++
 security/integrity/ima/ima.h                  |  1 +
 security/integrity/ima/ima_queue.c            | 36 ++++++++++++++++---
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 89670c5e7c8e..a651a3864dcf 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2345,6 +2345,10 @@ Kernel parameters
 			Use the canonical format for the binary runtime
 			measurements, instead of host native format.
 
+	ima_flush_htable  [IMA]
+			Flush the IMA hash table when deleting all the
+			staged measurement entries.
+
 	ima_hash=	[IMA]
 			Format: { md5 | sha1 | rmd160 | sha256 | sha384
 				   | sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 65db152a0a24..699b735dec7d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -340,6 +340,7 @@ extern atomic_long_t ima_num_entries[BINARY__LAST];
 extern atomic_long_t ima_num_violations;
 extern struct hlist_head __rcu *ima_htable;
 extern struct mutex ima_extend_list_mutex;
+extern bool ima_flush_htable;
 
 static inline unsigned int ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 50519ed837d4..f5c18acfbc43 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,6 +22,20 @@
 
 #define AUDIT_CAUSE_LEN_MAX 32
 
+bool ima_flush_htable;
+
+static int __init ima_flush_htable_setup(char *str)
+{
+	if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+		pr_warn("Hash table not enabled, ignoring request to flush\n");
+		return 1;
+	}
+
+	ima_flush_htable = true;
+	return 1;
+}
+__setup("ima_flush_htable", ima_flush_htable_setup);
+
 /* pre-allocated array of tpm_digest structures to extend a PCR */
 static struct tpm_digest *digests;
 
@@ -317,10 +331,11 @@ int ima_queue_stage(void)
 	return ret;
 }
 
-static void ima_queue_delete(struct list_head *head);
+static void ima_queue_delete(struct list_head *head, bool flush_htable);
 
 int ima_queue_staged_delete_all(void)
 {
+	struct hlist_head *old_queue = NULL;
 	LIST_HEAD(ima_measurements_trim);
 
 	mutex_lock(&ima_extend_list_mutex);
@@ -337,13 +352,26 @@ int ima_queue_staged_delete_all(void)
 	if (IS_ENABLED(CONFIG_IMA_KEXEC))
 		binary_runtime_size[BINARY_STAGED] = 0;
 
+	if (ima_flush_htable) {
+		old_queue = ima_alloc_replace_htable();
+		if (IS_ERR(old_queue)) {
+			mutex_unlock(&ima_extend_list_mutex);
+			return PTR_ERR(old_queue);
+		}
+	}
+
 	mutex_unlock(&ima_extend_list_mutex);
 
-	ima_queue_delete(&ima_measurements_trim);
+	if (ima_flush_htable) {
+		synchronize_rcu();
+		kfree(old_queue);
+	}
+
+	ima_queue_delete(&ima_measurements_trim, ima_flush_htable);
 	return 0;
 }
 
-static void ima_queue_delete(struct list_head *head)
+static void ima_queue_delete(struct list_head *head, bool flush_htable)
 {
 	struct ima_queue_entry *qe, *qe_tmp;
 	unsigned int i;
@@ -365,7 +393,7 @@ static void ima_queue_delete(struct list_head *head)
 		list_del(&qe->later);
 
 		/* No leak if condition is false, referenced by ima_htable. */
-		if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+		if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) || flush_htable) {
 			kfree(qe->entry->digests);
 			kfree(qe->entry);
 			kfree(qe);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 09/13] ima: Add support for staging measurements with prompt
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce the ability of staging the IMA measurement list and deleting them
with a prompt.

Staging means moving the current content of the measurement list to a
separate location, and allowing users to read and delete it. This causes
the measurement list to be atomically truncated before new measurements can
be added. Staging can be done only once at a time. In the event of kexec(),
staging is reverted and staged entries will be carried over to the new
kernel.

Introduce ascii_runtime_measurements_<algo>_staged and
binary_runtime_measurements_<algo>_staged interfaces to stage and delete
the measurements. Use 'echo A > <IMA interface>' and
'echo D > <IMA interface>' to respectively stage and delete the entire
measurements list. Locking of these interfaces is also mediated with a call
to _ima_measurements_open() and with ima_measurements_release().

Implement the staging functionality by introducing the new global
measurements list ima_measurements_staged, and ima_queue_stage() and
ima_queue_delete_staged_all() to respectively move measurements from the
current measurements list to the staged one, and to move staged
measurements to the ima_measurements_trim list for deletion. Introduce
ima_queue_delete() to delete the measurements.

Finally, introduce the BINARY_STAGED AND BINARY_FULL binary measurements
list types, to maintain the counters and the binary size of staged
measurements and the full measurements list (including entries that were
staged). BINARY still represents the current binary measurements list.

Use the binary size for the BINARY + BINARY_STAGED types in
ima_add_kexec_buffer(), since both measurements list types are copied to
the secondary kernel during kexec. Use BINARY_FULL in
ima_measure_kexec_event(), to generate a critical data record.

It should be noted that the BINARY_FULL counter is not passed through
kexec. Thus, the number of entries included in the kexec critical data
records refers to the entries since the previous kexec records.

Note: This code derives from the Alt-IMA Huawei project, whose license is
      GPL-2.0 OR MIT.

Link: https://github.com/linux-integrity/linux/issues/1
Suggested-by: Gregory Lumen <gregorylumen@linux.microsoft.com> (staging revert)
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/Kconfig     |  13 +++
 security/integrity/ima/ima.h       |   8 +-
 security/integrity/ima/ima_fs.c    | 167 ++++++++++++++++++++++++++---
 security/integrity/ima/ima_kexec.c |  22 +++-
 security/integrity/ima/ima_queue.c |  97 ++++++++++++++++-
 5 files changed, 286 insertions(+), 21 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 976e75f9b9ba..e714726f3384 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -332,4 +332,17 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
 	  If set to the default value of 0, an extra half page of memory for those
 	  additional measurements will be allocated.
 
+config IMA_STAGING
+	bool "Support for staging the measurements list"
+	default y
+	help
+	  Add support for staging the measurements list.
+
+	  It allows user space to stage the measurements list for deletion and
+	  to delete the staged measurements after confirmation.
+
+	  On kexec, staging is reverted and staged measurements are prepended
+	  to the current measurements list when measurements are copied to the
+	  secondary kernel.
+
 endif
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 97b7d6024b5d..65db152a0a24 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -30,9 +30,11 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
 /*
  * BINARY: current binary measurements list
+ * BINARY_STAGED: staged binary measurements list
+ * BINARY_FULL: binary measurements list since IMA init (lost after kexec)
  */
 enum binary_lists {
-	BINARY, BINARY__LAST
+	BINARY, BINARY_STAGED, BINARY_FULL, BINARY__LAST
 };
 
 /* digest size for IMA, fits SHA1 or MD5 */
@@ -125,6 +127,7 @@ struct ima_queue_entry {
 	struct ima_template_entry *entry;
 };
 extern struct list_head ima_measurements;	/* list of all measurements */
+extern struct list_head ima_measurements_staged; /* list of staged meas. */
 
 /* Some details preceding the binary serialized measurement list */
 struct ima_kexec_hdr {
@@ -314,6 +317,8 @@ struct ima_template_desc *ima_template_desc_current(void);
 struct ima_template_desc *ima_template_desc_buf(void);
 struct ima_template_desc *lookup_template_desc(const char *name);
 bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
+int ima_queue_stage(void);
+int ima_queue_staged_delete_all(void);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
@@ -334,6 +339,7 @@ extern spinlock_t ima_queue_lock;
 extern atomic_long_t ima_num_entries[BINARY__LAST];
 extern atomic_long_t ima_num_violations;
 extern struct hlist_head __rcu *ima_htable;
+extern struct mutex ima_extend_list_mutex;
 
 static inline unsigned int ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 7709a4576322..39d9128e9f22 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -24,6 +24,13 @@
 
 #include "ima.h"
 
+/*
+ * Requests:
+ * 'A\n': stage the entire measurements list
+ * 'D\n': delete all staged measurements
+ */
+#define STAGED_REQ_LENGTH 21
+
 static DEFINE_MUTEX(ima_write_mutex);
 static DEFINE_MUTEX(ima_measure_mutex);
 static long ima_measure_users;
@@ -97,6 +104,11 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
 	return _ima_measurements_start(m, pos, &ima_measurements);
 }
 
+static void *ima_measurements_staged_start(struct seq_file *m, loff_t *pos)
+{
+	return _ima_measurements_start(m, pos, &ima_measurements_staged);
+}
+
 static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
 				    struct list_head *head)
 {
@@ -118,6 +130,12 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
 	return _ima_measurements_next(m, v, pos, &ima_measurements);
 }
 
+static void *ima_measurements_staged_next(struct seq_file *m, void *v,
+					  loff_t *pos)
+{
+	return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
+}
+
 static void ima_measurements_stop(struct seq_file *m, void *v)
 {
 }
@@ -283,6 +301,68 @@ static const struct file_operations ima_measurements_ops = {
 	.release = ima_measurements_release,
 };
 
+static const struct seq_operations ima_measurments_staged_seqops = {
+	.start = ima_measurements_staged_start,
+	.next = ima_measurements_staged_next,
+	.stop = ima_measurements_stop,
+	.show = ima_measurements_show
+};
+
+static int ima_measurements_staged_open(struct inode *inode, struct file *file)
+{
+	return _ima_measurements_open(inode, file,
+				      &ima_measurments_staged_seqops);
+}
+
+static ssize_t ima_measurements_staged_write(struct file *file,
+					     const char __user *buf,
+					     size_t datalen, loff_t *ppos)
+{
+	char req[STAGED_REQ_LENGTH];
+	int ret;
+
+	if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
+		return -EINVAL;
+
+	if (copy_from_user(req, buf, datalen) != 0)
+		return -EFAULT;
+
+	if (req[datalen - 1] != '\n')
+		return -EINVAL;
+
+	req[datalen - 1] = '\0';
+
+	switch (req[0]) {
+	case 'A':
+		if (datalen != 2)
+			return -EINVAL;
+
+		ret = ima_queue_stage();
+		break;
+	case 'D':
+		if (datalen != 2)
+			return -EINVAL;
+
+		ret = ima_queue_staged_delete_all();
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (ret < 0)
+		return ret;
+
+	return datalen;
+}
+
+static const struct file_operations ima_measurements_staged_ops = {
+	.open = ima_measurements_staged_open,
+	.read = seq_read,
+	.write = ima_measurements_staged_write,
+	.llseek = seq_lseek,
+	.release = ima_measurements_release,
+};
+
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
 {
 	u32 i;
@@ -356,6 +436,28 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.release = ima_measurements_release,
 };
 
+static const struct seq_operations ima_ascii_measurements_staged_seqops = {
+	.start = ima_measurements_staged_start,
+	.next = ima_measurements_staged_next,
+	.stop = ima_measurements_stop,
+	.show = ima_ascii_measurements_show
+};
+
+static int ima_ascii_measurements_staged_open(struct inode *inode,
+					      struct file *file)
+{
+	return _ima_measurements_open(inode, file,
+				      &ima_ascii_measurements_staged_seqops);
+}
+
+static const struct file_operations ima_ascii_measurements_staged_ops = {
+	.open = ima_ascii_measurements_staged_open,
+	.read = seq_read,
+	.write = ima_measurements_staged_write,
+	.llseek = seq_lseek,
+	.release = ima_measurements_release,
+};
+
 static ssize_t ima_read_policy(char *path)
 {
 	void *data = NULL;
@@ -459,10 +561,21 @@ static const struct seq_operations ima_policy_seqops = {
 };
 #endif
 
-static int __init create_securityfs_measurement_lists(void)
+static int __init create_securityfs_measurement_lists(bool staging)
 {
+	const struct file_operations *ascii_ops = &ima_ascii_measurements_ops;
+	const struct file_operations *binary_ops = &ima_measurements_ops;
+	mode_t permissions = S_IRUSR | S_IRGRP;
+	const char *file_suffix = "";
 	int count = NR_BANKS(ima_tpm_chip);
 
+	if (staging) {
+		ascii_ops = &ima_ascii_measurements_staged_ops;
+		binary_ops = &ima_measurements_staged_ops;
+		file_suffix = "_staged";
+		permissions |= (S_IWUSR | S_IWGRP);
+	}
+
 	if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
 		count++;
 
@@ -473,29 +586,32 @@ static int __init create_securityfs_measurement_lists(void)
 
 		if (algo == HASH_ALGO__LAST)
 			snprintf(file_name, sizeof(file_name),
-				 "ascii_runtime_measurements_tpm_alg_%x",
-				 ima_tpm_chip->allocated_banks[i].alg_id);
+				 "ascii_runtime_measurements_tpm_alg_%x%s",
+				 ima_tpm_chip->allocated_banks[i].alg_id,
+				 file_suffix);
 		else
 			snprintf(file_name, sizeof(file_name),
-				 "ascii_runtime_measurements_%s",
-				 hash_algo_name[algo]);
-		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+				 "ascii_runtime_measurements_%s%s",
+				 hash_algo_name[algo], file_suffix);
+		dentry = securityfs_create_file(file_name, permissions,
 						ima_dir, (void *)(uintptr_t)i,
-						&ima_ascii_measurements_ops);
+						ascii_ops);
 		if (IS_ERR(dentry))
 			return PTR_ERR(dentry);
 
 		if (algo == HASH_ALGO__LAST)
 			snprintf(file_name, sizeof(file_name),
-				 "binary_runtime_measurements_tpm_alg_%x",
-				 ima_tpm_chip->allocated_banks[i].alg_id);
+				 "binary_runtime_measurements_tpm_alg_%x%s",
+				 ima_tpm_chip->allocated_banks[i].alg_id,
+				 file_suffix);
 		else
 			snprintf(file_name, sizeof(file_name),
-				 "binary_runtime_measurements_%s",
-				 hash_algo_name[algo]);
-		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+				 "binary_runtime_measurements_%s%s",
+				 hash_algo_name[algo], file_suffix);
+
+		dentry = securityfs_create_file(file_name, permissions,
 						ima_dir, (void *)(uintptr_t)i,
-						&ima_measurements_ops);
+						binary_ops);
 		if (IS_ERR(dentry))
 			return PTR_ERR(dentry);
 	}
@@ -503,6 +619,23 @@ static int __init create_securityfs_measurement_lists(void)
 	return 0;
 }
 
+static int __init create_securityfs_staging_links(void)
+{
+	struct dentry *dentry;
+
+	dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
+		ima_dir, "binary_runtime_measurements_sha1_staged", NULL);
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
+		ima_dir, "ascii_runtime_measurements_sha1_staged", NULL);
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	return 0;
+}
+
 /*
  * ima_open_policy: sequentialize access to the policy file
  */
@@ -595,7 +728,13 @@ int __init ima_fs_init(void)
 		goto out;
 	}
 
-	ret = create_securityfs_measurement_lists();
+	ret = create_securityfs_measurement_lists(false);
+	if (ret == 0 && IS_ENABLED(CONFIG_IMA_STAGING)) {
+		ret = create_securityfs_measurement_lists(true);
+		if (ret == 0)
+			ret = create_securityfs_staging_links();
+	}
+
 	if (ret != 0)
 		goto out;
 
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index d7d0fb639d99..d5503dd5cc9b 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -42,8 +42,8 @@ void ima_measure_kexec_event(const char *event_name)
 	long len;
 	int n;
 
-	buf_size = ima_get_binary_runtime_size(BINARY);
-	len = atomic_long_read(&ima_num_entries[BINARY]);
+	buf_size = ima_get_binary_runtime_size(BINARY_FULL);
+	len = atomic_long_read(&ima_num_entries[BINARY_FULL]);
 
 	n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
 		      "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
@@ -106,13 +106,26 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 
 	memset(&khdr, 0, sizeof(khdr));
 	khdr.version = 1;
-	/* This is an append-only list, no need to hold the RCU read lock */
-	list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
+	/* It can race with ima_queue_stage() and ima_queue_delete_staged(). */
+	mutex_lock(&ima_extend_list_mutex);
+
+	list_for_each_entry_rcu(qe, &ima_measurements_staged, later,
+				lockdep_is_held(&ima_extend_list_mutex)) {
 		ret = ima_dump_measurement(&khdr, qe);
 		if (ret < 0)
 			break;
 	}
 
+	list_for_each_entry_rcu(qe, &ima_measurements, later,
+				lockdep_is_held(&ima_extend_list_mutex)) {
+		if (!ret)
+			ret = ima_dump_measurement(&khdr, qe);
+		if (ret < 0)
+			break;
+	}
+
+	mutex_unlock(&ima_extend_list_mutex);
+
 	/*
 	 * fill in reserved space with some buffer details
 	 * (eg. version, buffer size, number of measurements)
@@ -167,6 +180,7 @@ void ima_add_kexec_buffer(struct kimage *image)
 		extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
 
 	binary_runtime_size = ima_get_binary_runtime_size(BINARY) +
+			      ima_get_binary_runtime_size(BINARY_STAGED) +
 			      extra_memory;
 
 	if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index b6d10dceb669..50519ed837d4 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -26,6 +26,7 @@
 static struct tpm_digest *digests;
 
 LIST_HEAD(ima_measurements);	/* list of all measurements */
+LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
 #ifdef CONFIG_IMA_KEXEC
 static unsigned long binary_runtime_size[BINARY__LAST];
 #else
@@ -45,11 +46,11 @@ atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
 /* key: inode (before secure-hashing a file) */
 struct hlist_head __rcu *ima_htable;
 
-/* mutex protects atomicity of extending measurement list
+/* mutex protects atomicity of extending and staging measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.
  */
-static DEFINE_MUTEX(ima_extend_list_mutex);
+DEFINE_MUTEX(ima_extend_list_mutex);
 
 /*
  * Used internally by the kernel to suspend measurements.
@@ -174,12 +175,16 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 				lockdep_is_held(&ima_extend_list_mutex));
 
 	atomic_long_inc(&ima_num_entries[BINARY]);
+	atomic_long_inc(&ima_num_entries[BINARY_FULL]);
+
 	if (update_htable) {
 		key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
 		hlist_add_head_rcu(&qe->hnext, &htable[key]);
 	}
 
 	ima_update_binary_runtime_size(entry, BINARY);
+	ima_update_binary_runtime_size(entry, BINARY_FULL);
+
 	return 0;
 }
 
@@ -280,6 +285,94 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 	return result;
 }
 
+int ima_queue_stage(void)
+{
+	int ret = 0;
+
+	mutex_lock(&ima_extend_list_mutex);
+	if (!list_empty(&ima_measurements_staged)) {
+		ret = -EEXIST;
+		goto out_unlock;
+	}
+
+	if (list_empty(&ima_measurements)) {
+		ret = -ENOENT;
+		goto out_unlock;
+	}
+
+	list_replace(&ima_measurements, &ima_measurements_staged);
+	INIT_LIST_HEAD(&ima_measurements);
+
+	atomic_long_set(&ima_num_entries[BINARY_STAGED],
+			atomic_long_read(&ima_num_entries[BINARY]));
+	atomic_long_set(&ima_num_entries[BINARY], 0);
+
+	if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
+		binary_runtime_size[BINARY_STAGED] =
+					binary_runtime_size[BINARY];
+		binary_runtime_size[BINARY] = 0;
+	}
+out_unlock:
+	mutex_unlock(&ima_extend_list_mutex);
+	return ret;
+}
+
+static void ima_queue_delete(struct list_head *head);
+
+int ima_queue_staged_delete_all(void)
+{
+	LIST_HEAD(ima_measurements_trim);
+
+	mutex_lock(&ima_extend_list_mutex);
+	if (list_empty(&ima_measurements_staged)) {
+		mutex_unlock(&ima_extend_list_mutex);
+		return -ENOENT;
+	}
+
+	list_replace(&ima_measurements_staged, &ima_measurements_trim);
+	INIT_LIST_HEAD(&ima_measurements_staged);
+
+	atomic_long_set(&ima_num_entries[BINARY_STAGED], 0);
+
+	if (IS_ENABLED(CONFIG_IMA_KEXEC))
+		binary_runtime_size[BINARY_STAGED] = 0;
+
+	mutex_unlock(&ima_extend_list_mutex);
+
+	ima_queue_delete(&ima_measurements_trim);
+	return 0;
+}
+
+static void ima_queue_delete(struct list_head *head)
+{
+	struct ima_queue_entry *qe, *qe_tmp;
+	unsigned int i;
+
+	list_for_each_entry_safe(qe, qe_tmp, head, later) {
+		/*
+		 * Safe to free template_data here without synchronize_rcu()
+		 * because the only htable reader, ima_lookup_digest_entry(),
+		 * accesses only entry->digests, not template_data. If new
+		 * htable readers are added that access template_data, a
+		 * synchronize_rcu() is required here.
+		 */
+		for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
+			kfree(qe->entry->template_data[i].data);
+			qe->entry->template_data[i].data = NULL;
+			qe->entry->template_data[i].len = 0;
+		}
+
+		list_del(&qe->later);
+
+		/* No leak if condition is false, referenced by ima_htable. */
+		if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+			kfree(qe->entry->digests);
+			kfree(qe->entry);
+			kfree(qe);
+		}
+	}
+}
+
 int ima_restore_measurement_entry(struct ima_template_entry *entry)
 {
 	int result = 0;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 08/13] ima: Introduce ima_dump_measurement()
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce ima_dump_measurement() to simplify the code of
ima_dump_measurement_list() and to avoid repeating the
ima_dump_measurement() code block if iteration occurs on multiple lists.

No functional change: only code moved to a separate function.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_kexec.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 44ebefbdcab0..d7d0fb639d99 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -80,6 +80,17 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
 	return 0;
 }
 
+static int ima_dump_measurement(struct ima_kexec_hdr *khdr,
+				struct ima_queue_entry *qe)
+{
+	if (ima_kexec_file.count >= ima_kexec_file.size)
+		return -EINVAL;
+
+	khdr->count++;
+	ima_measurements_show(&ima_kexec_file, qe);
+	return 0;
+}
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
@@ -97,13 +108,9 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 	khdr.version = 1;
 	/* This is an append-only list, no need to hold the RCU read lock */
 	list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
-		if (ima_kexec_file.count < ima_kexec_file.size) {
-			khdr.count++;
-			ima_measurements_show(&ima_kexec_file, qe);
-		} else {
-			ret = -EINVAL;
+		ret = ima_dump_measurement(&khdr, qe);
+		if (ret < 0)
 			break;
-		}
 	}
 
 	/*
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 07/13] ima: Use snprintf() in create_securityfs_measurement_lists
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Use the more secure snprintf() function (accepting the buffer size) in
create_securityfs_measurement_lists().

No functional change: sprintf() and snprintf() have the same behavior.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 68edea7139d5..7709a4576322 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -472,11 +472,13 @@ static int __init create_securityfs_measurement_lists(void)
 		struct dentry *dentry;
 
 		if (algo == HASH_ALGO__LAST)
-			sprintf(file_name, "ascii_runtime_measurements_tpm_alg_%x",
-				ima_tpm_chip->allocated_banks[i].alg_id);
+			snprintf(file_name, sizeof(file_name),
+				 "ascii_runtime_measurements_tpm_alg_%x",
+				 ima_tpm_chip->allocated_banks[i].alg_id);
 		else
-			sprintf(file_name, "ascii_runtime_measurements_%s",
-				hash_algo_name[algo]);
+			snprintf(file_name, sizeof(file_name),
+				 "ascii_runtime_measurements_%s",
+				 hash_algo_name[algo]);
 		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
 						ima_dir, (void *)(uintptr_t)i,
 						&ima_ascii_measurements_ops);
@@ -484,11 +486,13 @@ static int __init create_securityfs_measurement_lists(void)
 			return PTR_ERR(dentry);
 
 		if (algo == HASH_ALGO__LAST)
-			sprintf(file_name, "binary_runtime_measurements_tpm_alg_%x",
-				ima_tpm_chip->allocated_banks[i].alg_id);
+			snprintf(file_name, sizeof(file_name),
+				 "binary_runtime_measurements_tpm_alg_%x",
+				 ima_tpm_chip->allocated_banks[i].alg_id);
 		else
-			sprintf(file_name, "binary_runtime_measurements_%s",
-				hash_algo_name[algo]);
+			snprintf(file_name, sizeof(file_name),
+				 "binary_runtime_measurements_%s",
+				 hash_algo_name[algo]);
 		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
 						ima_dir, (void *)(uintptr_t)i,
 						&ima_measurements_ops);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 06/13] ima: Mediate open/release method of the measurements list
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce the ima_measure_users counter, to implement a semaphore-like
locking scheme where the binary and ASCII measurements list interfaces can
be concurrently open by multiple readers, or alternatively by a single
writer.

A semaphore cannot be used because the kernel cannot return to user space
with a lock held.

Introduce the ima_measure_lock() and ima_measure_unlock() primitives, to
respectively lock/unlock the interfaces (safely with the ima_measure_users
counter, without holding a lock).

Finally, introduce _ima_measurements_open() to lock the interface before
seq_open(), and call it from ima_measurements_open() and
ima_ascii_measurements_open(). And, introduce ima_measurements_release(),
to unlock the interface.

Require CAP_SYS_ADMIN if the interface is opened for write (not possible
for the current measurements interfaces, since they only have read
permission).

No functional changes: multiple readers are allowed as before.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 71 +++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 9a8dba14d82a..68edea7139d5 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -25,6 +25,8 @@
 #include "ima.h"
 
 static DEFINE_MUTEX(ima_write_mutex);
+static DEFINE_MUTEX(ima_measure_mutex);
+static long ima_measure_users;
 
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
@@ -209,16 +211,76 @@ static const struct seq_operations ima_measurments_seqops = {
 	.show = ima_measurements_show
 };
 
+static int ima_measure_lock(bool write)
+{
+	mutex_lock(&ima_measure_mutex);
+	if ((write && ima_measure_users != 0) ||
+	    (!write && ima_measure_users < 0)) {
+		mutex_unlock(&ima_measure_mutex);
+		return -EBUSY;
+	}
+
+	if (write)
+		ima_measure_users--;
+	else
+		ima_measure_users++;
+	mutex_unlock(&ima_measure_mutex);
+	return 0;
+}
+
+static void ima_measure_unlock(bool write)
+{
+	mutex_lock(&ima_measure_mutex);
+	if (write)
+		ima_measure_users++;
+	else
+		ima_measure_users--;
+	mutex_unlock(&ima_measure_mutex);
+}
+
+static int _ima_measurements_open(struct inode *inode, struct file *file,
+				  const struct seq_operations *seq_ops)
+{
+	bool write = (file->f_mode & FMODE_WRITE);
+	int ret;
+
+	if (write && !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	ret = ima_measure_lock(write);
+	if (ret < 0)
+		return ret;
+
+	ret = seq_open(file, seq_ops);
+	if (ret < 0)
+		ima_measure_unlock(write);
+
+	return ret;
+}
+
 static int ima_measurements_open(struct inode *inode, struct file *file)
 {
-	return seq_open(file, &ima_measurments_seqops);
+	return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static int ima_measurements_release(struct inode *inode, struct file *file)
+{
+	bool write = (file->f_mode & FMODE_WRITE);
+	int ret;
+
+	/* seq_release() always returns zero. */
+	ret = seq_release(inode, file);
+
+	ima_measure_unlock(write);
+
+	return ret;
 }
 
 static const struct file_operations ima_measurements_ops = {
 	.open = ima_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-	.release = seq_release,
+	.release = ima_measurements_release,
 };
 
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -283,14 +345,15 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
 
 static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
 {
-	return seq_open(file, &ima_ascii_measurements_seqops);
+	return _ima_measurements_open(inode, file,
+				      &ima_ascii_measurements_seqops);
 }
 
 static const struct file_operations ima_ascii_measurements_ops = {
 	.open = ima_ascii_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-	.release = seq_release,
+	.release = ima_measurements_release,
 };
 
 static ssize_t ima_read_policy(char *path)
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 05/13] ima: Introduce _ima_measurements_start() and _ima_measurements_next()
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce _ima_measurements_start() and _ima_measurements_next(), renamed
from ima_measurements_start() and ima_measurements_next(), to include the
list head as an additional parameter, so that iteration on different lists
can be implemented by calling those functions.

No functional change: ima_measurements_start() and ima_measurements_next()
pass the ima_measurements list head, used before.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 79b0f287c668..9a8dba14d82a 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -72,14 +72,15 @@ static const struct file_operations ima_measurements_count_ops = {
 };
 
 /* returns pointer to hlist_node */
-static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
+static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
+				     struct list_head *head)
 {
 	loff_t l = *pos;
 	struct ima_queue_entry *qe;
 
 	/* we need a lock since pos could point beyond last element */
 	rcu_read_lock();
-	list_for_each_entry_rcu(qe, &ima_measurements, later) {
+	list_for_each_entry_rcu(qe, head, later) {
 		if (!l--) {
 			rcu_read_unlock();
 			return qe;
@@ -89,7 +90,13 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
 	return NULL;
 }
 
-static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
+static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
+{
+	return _ima_measurements_start(m, pos, &ima_measurements);
+}
+
+static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
+				    struct list_head *head)
 {
 	struct ima_queue_entry *qe = v;
 
@@ -101,7 +108,12 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
 	rcu_read_unlock();
 	(*pos)++;
 
-	return (&qe->later == &ima_measurements) ? NULL : qe;
+	return (&qe->later == head) ? NULL : qe;
+}
+
+static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	return _ima_measurements_next(m, v, pos, &ima_measurements);
 }
 
 static void ima_measurements_stop(struct seq_file *m, void *v)
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 04/13] ima: Introduce per binary measurements list type binary_runtime_size value
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Make binary_runtime_size as an array, to have separate counters per binary
measurements list type. Currently, define the BINARY type for the existing
binary measurements list.

Introduce ima_update_binary_runtime_size() to facilitate updating a
binary_runtime_size value with a given binary measurement list type.

Also add the binary measurements list type parameter to
ima_get_binary_runtime_size(), to retrieve the desired value. Retrieving
the value is now done under the ima_extend_list_mutex, since there can be
concurrent updates.

No functional change (except for the mutex usage, that fixes the
concurrency issue): the BINARY array element is equivalent to the old
binary_runtime_size.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h       |  2 +-
 security/integrity/ima/ima_kexec.c |  5 ++--
 security/integrity/ima/ima_queue.c | 40 +++++++++++++++++++++---------
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 199237e2d2e3..97b7d6024b5d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -318,7 +318,7 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
 int __init ima_init_htable(void);
-unsigned long ima_get_binary_runtime_size(void);
+unsigned long ima_get_binary_runtime_size(enum binary_lists binary_list);
 int ima_init_template(void);
 void ima_init_template_list(void);
 int __init ima_init_digests(void);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 40962dc0ca86..44ebefbdcab0 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -42,7 +42,7 @@ void ima_measure_kexec_event(const char *event_name)
 	long len;
 	int n;
 
-	buf_size = ima_get_binary_runtime_size();
+	buf_size = ima_get_binary_runtime_size(BINARY);
 	len = atomic_long_read(&ima_num_entries[BINARY]);
 
 	n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
@@ -159,7 +159,8 @@ void ima_add_kexec_buffer(struct kimage *image)
 	else
 		extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
 
-	binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
+	binary_runtime_size = ima_get_binary_runtime_size(BINARY) +
+			      extra_memory;
 
 	if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
 		kexec_segment_size = ULONG_MAX;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 952172a4905d..b6d10dceb669 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -27,9 +27,11 @@ static struct tpm_digest *digests;
 
 LIST_HEAD(ima_measurements);	/* list of all measurements */
 #ifdef CONFIG_IMA_KEXEC
-static unsigned long binary_runtime_size;
+static unsigned long binary_runtime_size[BINARY__LAST];
 #else
-static unsigned long binary_runtime_size = ULONG_MAX;
+static unsigned long binary_runtime_size[BINARY__LAST] = {
+	[0 ... BINARY__LAST - 1] = ULONG_MAX
+};
 #endif
 
 /* num of stored measurements in the list */
@@ -131,6 +133,20 @@ static int get_binary_runtime_size(struct ima_template_entry *entry)
 	return size;
 }
 
+static void ima_update_binary_runtime_size(struct ima_template_entry *entry,
+					   enum binary_lists binary_list)
+{
+	int size;
+
+	if (binary_runtime_size[binary_list] == ULONG_MAX)
+		return;
+
+	size = get_binary_runtime_size(entry);
+	binary_runtime_size[binary_list] =
+		(binary_runtime_size[binary_list] < ULONG_MAX - size) ?
+		binary_runtime_size[binary_list] + size : ULONG_MAX;
+}
+
 /* ima_add_template_entry helper function:
  * - Add template entry to the measurement list and hash table, for
  *   all entries except those carried across kexec.
@@ -163,13 +179,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		hlist_add_head_rcu(&qe->hnext, &htable[key]);
 	}
 
-	if (binary_runtime_size != ULONG_MAX) {
-		int size;
-
-		size = get_binary_runtime_size(entry);
-		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
-		     binary_runtime_size + size : ULONG_MAX;
-	}
+	ima_update_binary_runtime_size(entry, BINARY);
 	return 0;
 }
 
@@ -178,12 +188,18 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
  * entire binary_runtime_measurement list, including the ima_kexec_hdr
  * structure.
  */
-unsigned long ima_get_binary_runtime_size(void)
+unsigned long ima_get_binary_runtime_size(enum binary_lists binary_list)
 {
-	if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
+	unsigned long val;
+
+	mutex_lock(&ima_extend_list_mutex);
+	val = binary_runtime_size[binary_list];
+	mutex_unlock(&ima_extend_list_mutex);
+
+	if (val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
 		return ULONG_MAX;
 	else
-		return binary_runtime_size + sizeof(struct ima_kexec_hdr);
+		return val + sizeof(struct ima_kexec_hdr);
 }
 
 static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 03/13] ima: Introduce per binary measurements list type ima_num_entries counter
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

Make ima_num_entries as an array, to have separate counters per binary
measurements list type. Currently, define the BINARY type for the existing
binary measurements list.

No functional change: the BINARY type is equivalent to the value without
the array.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h       | 9 ++++++++-
 security/integrity/ima/ima_fs.c    | 3 +--
 security/integrity/ima/ima_kexec.c | 2 +-
 security/integrity/ima/ima_queue.c | 7 +++++--
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 9cdc4c5afd3b..199237e2d2e3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -28,6 +28,13 @@ enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
 		     IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
 enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
+/*
+ * BINARY: current binary measurements list
+ */
+enum binary_lists {
+	BINARY, BINARY__LAST
+};
+
 /* digest size for IMA, fits SHA1 or MD5 */
 #define IMA_DIGEST_SIZE		SHA1_DIGEST_SIZE
 #define IMA_EVENT_NAME_LEN_MAX	255
@@ -324,7 +331,7 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
  */
 extern spinlock_t ima_queue_lock;
 
-extern atomic_long_t ima_num_entries;
+extern atomic_long_t ima_num_entries[BINARY__LAST];
 extern atomic_long_t ima_num_violations;
 extern struct hlist_head __rcu *ima_htable;
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index aaa460d70ff7..79b0f287c668 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -63,8 +63,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
 					   char __user *buf,
 					   size_t count, loff_t *ppos)
 {
-	return ima_show_counter(buf, count, ppos, &ima_num_entries);
-
+	return ima_show_counter(buf, count, ppos, &ima_num_entries[BINARY]);
 }
 
 static const struct file_operations ima_measurements_count_ops = {
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 5801649fbbef..40962dc0ca86 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -43,7 +43,7 @@ void ima_measure_kexec_event(const char *event_name)
 	int n;
 
 	buf_size = ima_get_binary_runtime_size();
-	len = atomic_long_read(&ima_num_entries);
+	len = atomic_long_read(&ima_num_entries[BINARY]);
 
 	n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
 		      "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 41f4941ceaad..952172a4905d 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -33,7 +33,10 @@ static unsigned long binary_runtime_size = ULONG_MAX;
 #endif
 
 /* num of stored measurements in the list */
-atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
+atomic_long_t ima_num_entries[BINARY__LAST] = {
+	[0 ... BINARY__LAST - 1] = ATOMIC_LONG_INIT(0)
+};
+
 /* num of violations in the list */
 atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
 
@@ -154,7 +157,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 	htable = rcu_dereference_protected(ima_htable,
 				lockdep_is_held(&ima_extend_list_mutex));
 
-	atomic_long_inc(&ima_num_entries);
+	atomic_long_inc(&ima_num_entries[BINARY]);
 	if (update_htable) {
 		key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
 		hlist_add_head_rcu(&qe->hnext, &htable[key]);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 02/13] ima: Replace static htable queue with dynamically allocated array
From: Roberto Sassu @ 2026-03-26 17:30 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

The IMA hash table is a fixed-size array of hlist_head buckets:

    struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];

IMA_MEASURE_HTABLE_SIZE is (1 << IMA_HASH_BITS) = 1024 buckets, each a
struct hlist_head (one pointer, 8 bytes on 64-bit). That is 8 KiB allocated
in BSS for every kernel, regardless of whether IMA is ever used, and
regardless of how many measurements are actually made.

Replace the fixed-size array with a RCU-protected pointer to a dynamically
allocated array that is initialized in ima_init_htable(), which is called
from ima_init() during early boot. ima_init_htable() calls the static
function ima_alloc_replace_htable() which, other than initializing the hash
table the first time, can also hot-swap the existing hash table with a
blank one.

The allocation in ima_alloc_replace_htable() uses kcalloc() so the buckets
are zero-initialised (equivalent to HLIST_HEAD_INIT { .first = NULL }).
Callers of ima_alloc_replace_htable() must call synchronize_rcu() and free
the returned hash table.

Finally, access the hash table with rcu_dereference() in
ima_lookup_digest_entry() (reader side) and with
rcu_dereference_protected() in ima_add_digest_entry() (writer side).

No functional change: bucket count, hash function, and all locking remain
identical.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h       |  3 +-
 security/integrity/ima/ima_init.c  |  5 ++++
 security/integrity/ima/ima_queue.c | 48 ++++++++++++++++++++++++++----
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d759988fde45..9cdc4c5afd3b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -310,6 +310,7 @@ bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
+int __init ima_init_htable(void);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
 void ima_init_template_list(void);
@@ -325,7 +326,7 @@ extern spinlock_t ima_queue_lock;
 
 extern atomic_long_t ima_num_entries;
 extern atomic_long_t ima_num_violations;
-extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
+extern struct hlist_head __rcu *ima_htable;
 
 static inline unsigned int ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..7e0aa09a12e6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -140,6 +140,11 @@ int __init ima_init(void)
 	rc = ima_init_digests();
 	if (rc != 0)
 		return rc;
+
+	rc = ima_init_htable();
+	if (rc != 0)
+		return rc;
+
 	rc = ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	if (rc != 0)
 		return rc;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 1f6515f7d015..41f4941ceaad 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -38,9 +38,7 @@ atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
 atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
 
 /* key: inode (before secure-hashing a file) */
-struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE] = {
-	[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
-};
+struct hlist_head __rcu *ima_htable;
 
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
@@ -54,17 +52,53 @@ static DEFINE_MUTEX(ima_extend_list_mutex);
  */
 static bool ima_measurements_suspended;
 
+/* Callers must call synchronize_rcu() and free the hash table. */
+static struct hlist_head *ima_alloc_replace_htable(void)
+{
+	struct hlist_head *old_htable, *new_htable;
+
+	/* Initializing to zeros is equivalent to call HLIST_HEAD_INIT. */
+	new_htable = kcalloc(IMA_MEASURE_HTABLE_SIZE, sizeof(struct hlist_head),
+			     GFP_KERNEL);
+	if (!new_htable)
+		return ERR_PTR(-ENOMEM);
+
+	old_htable = rcu_replace_pointer(ima_htable, new_htable,
+				lockdep_is_held(&ima_extend_list_mutex));
+
+	return old_htable;
+}
+
+int __init ima_init_htable(void)
+{
+	struct hlist_head *old_htable;
+
+	mutex_lock(&ima_extend_list_mutex);
+	old_htable = ima_alloc_replace_htable();
+	mutex_unlock(&ima_extend_list_mutex);
+
+	if (IS_ERR(old_htable))
+		return PTR_ERR(old_htable);
+
+	/* Synchronize_rcu() and kfree() not necessary, only for robustness. */
+	synchronize_rcu();
+	kfree(old_htable);
+	return 0;
+}
+
 /* lookup up the digest value in the hash table, and return the entry */
 static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 						       int pcr)
 {
 	struct ima_queue_entry *qe, *ret = NULL;
+	struct hlist_head *htable;
 	unsigned int key;
 	int rc;
 
 	key = ima_hash_key(digest_value);
 	rcu_read_lock();
-	hlist_for_each_entry_rcu(qe, &ima_htable[key], hnext) {
+	htable = rcu_dereference(ima_htable);
+	hlist_for_each_entry_rcu(qe, &htable[key], hnext) {
 		rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
 			    digest_value, hash_digest_size[ima_hash_algo]);
 		if ((rc == 0) && (qe->entry->pcr == pcr)) {
@@ -104,6 +138,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 				bool update_htable)
 {
 	struct ima_queue_entry *qe;
+	struct hlist_head *htable;
 	unsigned int key;
 
 	qe = kmalloc_obj(*qe);
@@ -116,10 +151,13 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 	INIT_LIST_HEAD(&qe->later);
 	list_add_tail_rcu(&qe->later, &ima_measurements);
 
+	htable = rcu_dereference_protected(ima_htable,
+				lockdep_is_held(&ima_extend_list_mutex));
+
 	atomic_long_inc(&ima_num_entries);
 	if (update_htable) {
 		key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
-		hlist_add_head_rcu(&qe->hnext, &ima_htable[key]);
+		hlist_add_head_rcu(&qe->hnext, &htable[key]);
 	}
 
 	if (binary_runtime_size != ULONG_MAX) {
-- 
2.43.0


^ permalink raw reply related

* [PATCH v4 01/13] ima: Remove ima_h_table structure
From: Roberto Sassu @ 2026-03-26 17:29 UTC (permalink / raw)
  To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260326173011.1191815-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

With the upcoming change of dynamically allocating and replacing the hash
table, the ima_h_table structure would have been replaced with a new one.

However, since the ima_h_table structure contains also the counters for
number of measurements entries and violations, we would have needed to
preserve their values in the new ima_h_table structure.

Instead, separate those counters from the hash table, remove the
ima_h_table structure, and just replace the hash table pointer.

Finally, rename ima_show_htable_value(), ima_show_htable_violations()
and ima_htable_violations_ops respectively to ima_show_counter(),
ima_show_num_violations() and ima_num_violations_ops.

Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h       |  9 +++------
 security/integrity/ima/ima_api.c   |  2 +-
 security/integrity/ima/ima_fs.c    | 19 +++++++++----------
 security/integrity/ima/ima_kexec.c |  2 +-
 security/integrity/ima/ima_queue.c | 17 ++++++++++-------
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 0eea02ff04df..d759988fde45 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -323,12 +323,9 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
  */
 extern spinlock_t ima_queue_lock;
 
-struct ima_h_table {
-	atomic_long_t len;	/* number of stored measurements in the list */
-	atomic_long_t violations;
-	struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
-};
-extern struct ima_h_table ima_htable;
+extern atomic_long_t ima_num_entries;
+extern atomic_long_t ima_num_violations;
+extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
 
 static inline unsigned int ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 0916f24f005f..122d127e108d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -146,7 +146,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 	int result;
 
 	/* can overflow, only indicator */
-	atomic_long_inc(&ima_htable.violations);
+	atomic_long_inc(&ima_num_violations);
 
 	result = ima_alloc_init_template(&event_data, &entry, NULL);
 	if (result < 0) {
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index ca4931a95098..aaa460d70ff7 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -38,8 +38,8 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
 
 static int valid_policy = 1;
 
-static ssize_t ima_show_htable_value(char __user *buf, size_t count,
-				     loff_t *ppos, atomic_long_t *val)
+static ssize_t ima_show_counter(char __user *buf, size_t count, loff_t *ppos,
+				atomic_long_t *val)
 {
 	char tmpbuf[32];	/* greater than largest 'long' string value */
 	ssize_t len;
@@ -48,15 +48,14 @@ static ssize_t ima_show_htable_value(char __user *buf, size_t count,
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
 }
 
-static ssize_t ima_show_htable_violations(struct file *filp,
-					  char __user *buf,
-					  size_t count, loff_t *ppos)
+static ssize_t ima_show_num_violations(struct file *filp, char __user *buf,
+				       size_t count, loff_t *ppos)
 {
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.violations);
+	return ima_show_counter(buf, count, ppos, &ima_num_violations);
 }
 
-static const struct file_operations ima_htable_violations_ops = {
-	.read = ima_show_htable_violations,
+static const struct file_operations ima_num_violations_ops = {
+	.read = ima_show_num_violations,
 	.llseek = generic_file_llseek,
 };
 
@@ -64,7 +63,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
 					   char __user *buf,
 					   size_t count, loff_t *ppos)
 {
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
+	return ima_show_counter(buf, count, ppos, &ima_num_entries);
 
 }
 
@@ -545,7 +544,7 @@ int __init ima_fs_init(void)
 	}
 
 	dentry = securityfs_create_file("violations", S_IRUSR | S_IRGRP,
-				   ima_dir, NULL, &ima_htable_violations_ops);
+				   ima_dir, NULL, &ima_num_violations_ops);
 	if (IS_ERR(dentry)) {
 		ret = PTR_ERR(dentry);
 		goto out;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 36a34c54de58..5801649fbbef 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -43,7 +43,7 @@ void ima_measure_kexec_event(const char *event_name)
 	int n;
 
 	buf_size = ima_get_binary_runtime_size();
-	len = atomic_long_read(&ima_htable.len);
+	len = atomic_long_read(&ima_num_entries);
 
 	n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
 		      "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 319522450854..1f6515f7d015 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -32,11 +32,14 @@ static unsigned long binary_runtime_size;
 static unsigned long binary_runtime_size = ULONG_MAX;
 #endif
 
+/* num of stored measurements in the list */
+atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
+/* num of violations in the list */
+atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
+
 /* key: inode (before secure-hashing a file) */
-struct ima_h_table ima_htable = {
-	.len = ATOMIC_LONG_INIT(0),
-	.violations = ATOMIC_LONG_INIT(0),
-	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
+struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE] = {
+	[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
 /* mutex protects atomicity of extending measurement list
@@ -61,7 +64,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 
 	key = ima_hash_key(digest_value);
 	rcu_read_lock();
-	hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
+	hlist_for_each_entry_rcu(qe, &ima_htable[key], hnext) {
 		rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
 			    digest_value, hash_digest_size[ima_hash_algo]);
 		if ((rc == 0) && (qe->entry->pcr == pcr)) {
@@ -113,10 +116,10 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 	INIT_LIST_HEAD(&qe->later);
 	list_add_tail_rcu(&qe->later, &ima_measurements);
 
-	atomic_long_inc(&ima_htable.len);
+	atomic_long_inc(&ima_num_entries);
 	if (update_htable) {
 		key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
-		hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
+		hlist_add_head_rcu(&qe->hnext, &ima_htable[key]);
 	}
 
 	if (binary_runtime_size != ULONG_MAX) {
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Babu Moger @ 2026-03-26 17:12 UTC (permalink / raw)
  To: Reinette Chatre, corbet, tony.luck, Dave.Martin, james.morse,
	tglx, mingo, bp, dave.hansen
  Cc: skhan, x86, hpa, peterz, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
	rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
	paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
	xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
	elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
	eranian, peternewman
In-Reply-To: <14a8ad0a-e842-4268-871a-0762f1169e03@intel.com>

Hi Reinette,

Thanks for the review comments. Will address one by one.

On 3/24/26 17:51, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/12/26 1:36 PM, Babu Moger wrote:
>> This series adds support for Privilege-Level Zero Association (PLZA) to the
>> resctrl subsystem. PLZA is an AMD feature that allows specifying a CLOSID
>> and/or RMID for execution in kernel mode (privilege level zero), so that
>> kernel work is not subject to the same resource constrains as the current
>> user-space task. This avoids kernel operations being aggressively throttled
>> when a task's memory bandwidth is heavily limited.
>>
>> The feature documentation is not yet publicly available, but it is expected
>> to be released in the next few weeks. In the meantime, a brief description
>> of the features is provided below.
>>
>> Privilege Level Zero Association (PLZA)
>>
>> Privilege Level Zero Association (PLZA) allows the hardware to
>> automatically associate execution in Privilege Level Zero (CPL=0) with a
>> specific COS (Class of Service) and/or RMID (Resource Monitoring
>> Identifier). The QoS feature set already has a mechanism to associate
>> execution on each logical processor with an RMID or COS. PLZA allows the
>> system to override this per-thread association for a thread that is
>> executing with CPL=0.
>> ------------------------------------------------------------------------
>>
>> The series introduces the feature in a way that supports the interface in
>> a generic manner to accomodate MPAM or other vendor specific implimentation.
>>
>> Below is the detailed requirements provided by Reinette:
>> https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
> Our discussion considered how resctrl could support PLZA in a generic way while
> also preparing to support MPAM's variants and how PLZA may evolve to have similar
> capabilities when considering the capabilities of its registers.
>
> This does not mean that your work needs to implement everything that was discussed.
> Instead, this work is expected to just support what PLZA is capable of today but
> do so in a way that the future enhancements could be added to.
>
> This series is quite difficult to follow since it appears to implement a full
> featured generic interface while PLZA cannot take advantage of it.
>
> Could you please simplify this work to focus on just enabling PLZA and only
> add interfaces needed to do so?
Sure. Will try. Lets continue the discussion.
>
>> Summary:
>> 1. Kernel-mode/PLZA controls and status should be exposed under the resctrl
>>     info directory:/sys/fs/resctrl/info/, not as a separate or arch-specific path.
>>
>> 2. Add two info files
>>
>>   a. kernel_mode
>>      Purpose: Control how resource allocation and monitoring apply in kernel mode
>>      (e.g. inherit from task vs global assign).
>>
>>      Read: List supported modes and show current one (e.g. with [brackets]).
>>      Write: Set current mode by name (e.g. inherit_ctrl_and_mon, global_assign_ctrl_assign_mon).
>>
>> b. kernel_mode_assignment
>>
>>     Purpose: When a “global assign” kernel mode is active, specify which resctrl group
>>     (CLOSID/RMID) is used for kernel work.
>>
>>     Read: Show the assigned group in a path-like form (e.g. //, ctrl1//, ctrl1/mon1/).
>>     Write: Assign or clear the group used for kernel mode (and optionally clear with an empty write).
>>
>> The patches are based on top of commit (v7.0.0-rc3)
>> 839e91ce3f41b (tip/master) Merge branch into tip/master: 'x86/tdx'
>> ------------------------------------------------------------------------
>>
>> Examples: kernel_mode and kernel_mode_assignment
>>
>> All paths below are under /sys/fs/resctrl/ (e.g. info/kernel_mode means
>> /sys/fs/resctrl/info/kernel_mode). Resctrl must be mounted and the platform
>> must support the relevant modes (e.g. AMD with PLZA).
>>
>> 1) kernel_mode — show and set the current kernel mode
>>
>>     Read supported modes and which one is active (current in brackets):
>>
>>       $ cat info/kernel_mode
>>       [inherit_ctrl_and_mon]
>>       global_assign_ctrl_inherit_mon
>>       global_assign_ctrl_assign_mon
>>
>>     Set the active mode (e.g. use one CLOSID+RMID for all kernel work):
>>
>>       $ echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>>       $ cat info/kernel_mode
>>       inherit_ctrl_and_mon
>>       global_assign_ctrl_inherit_mon
>>       [global_assign_ctrl_assign_mon]
>>
>>     Mode meanings:
>>     - inherit_ctrl_and_mon: kernel uses same CLOSID/RMID as the current task (default).
>>     - global_assign_ctrl_inherit_mon: one CLOSID for all kernel work; RMID inherited from user.
>>     - global_assign_ctrl_assign_mon: one resource group (CLOSID+RMID) for all kernel work.
>>
>> 2) kernel_mode_assignment — show and set which group is used for kernel work
>>
>>     Only relevant when kernel_mode is not "inherit_ctrl_and_mon". Read the
> To help with future usages please connect visibility of this file with the mode in
> info/kernel_mode. This helps us to support future modes with other resctrl files, possible
> within each resource group.
> Specifically, kernel_mode_assignment is not visible to user space if mode is "inherit_ctrl_and_mon",
> while it is visible when mode is global_assign_ctrl_inherit_mon or global_assign_ctrl_assign_mon.

Sure. Will do.

>
>>     currently assigned group (path format is "CTRL_MON/MON/"):
> The format depends on the mode, right? If the mode is "global_assign_ctrl_inherit_mon"
> then it should only contain a control group, alternatively, if the mode is
> "global_assign_ctrl_assign_mon" then it contains control and mon group. This gives
> resctrl future flexibility to change format for future modes.

This can be done both ways.  Whole purpose of these groups is to get 
CLOSID and RMID to enable PLZA. User can echo CTRL_MON or MON group to 
kernel_mode_assignment in any of the modes.  We can decide what needs to 
be updated in MSR (PQR_PLZA_ASSOC) based on what kernel mode is selected.


>
> We should also consider the scenario when it is a "monitoring only" system, which can
> happen independent from what hardware actually supports, for example, if user boots
> with "rdt=!l3cat,!l2cat,!mba,!smba". In this case I assume CLOS should just always be
> zero and thus only "default control group" is accepted?

Yes.  It depends on how we want to implement like we mentioned above.


>
>>       $ cat info/kernel_mode_assignment
>>       //
>>
>>     "//" means the default CTRL_MON group is assigned. Assign a specific
>>     group instead (e.g. a CTRL_MON group "ctrl1", or a MON group "mon1" under it):
>>
>>       $ echo "ctrl1//" > info/kernel_mode_assignment
>>       $ cat info/kernel_mode_assignment
>>       ctrl1//
>>
>>       $ echo "ctrl1/mon1/" > info/kernel_mode_assignment
>>       $ cat info/kernel_mode_assignment
>>       ctrl1/mon1/
>>
>>     Clear the assignment (no dedicated group for kernel work):
>>
>>       $ echo >> info/kernel_mode_assignment
>>       $ cat info/kernel_mode_assignment
>>       Kmode is not configured
> This does not look right. Would this not create a conflict between info/kernel_mode
> and info/kernel_mode_assignment about what the current mode is? The way I see it
> info/kernel_mode_assignment must always contain a valid group.
Yes.  We can do that.
>
>>     Errors (e.g. invalid group name or unsupported mode) are reported in
>>     info/last_cmd_status.
>>
>> ---
>>
>> v2:
>>       This is similar to RFC with new proposal. Names of the some interfaces
>>       are not final. Lets fix that later as we move forward.
>>
>>       Separated the two features: Global Bandwidth Enforcement (GLBE) and
>>       Privilege Level Zero Association (PLZA).
>>   
>>       This series only adds support for PLZA.
>>
>>       Used the name of the feature as kmode instead of PLZA. That can be changed as well.
>>
>>       Tony suggested using global variables to store the kernel mode
>>       CLOSID and RMID. However, the kernel mode CLOSID and RMID are
>>       coming from rdtgroup structure with the new interface. Accessing
>>       them requires holding the associated lock, which would make the
>>       context switch path unnecessarily expensive. So, dropped the idea.
>>       https://lore.kernel.org/lkml/aXuxVSbk1GR2ttzF@agluck-desk3/
>>       Let me know if there are other ways to optimize this.
> I do not see why the context switch path needs to be touched at all with this
> implementation. Since PLZA only supports global assignment does it not mean that resctrl
> only needs to update PQR_PLZA_ASSOC when user writes to info/kernel_mode and
> info/kernel_mode_assignment?

Each thread has an MSR to configure whether to associate privilege level 
zero execution with a separate COS and/or RMID, and the value of the COS 
and/or RMID.  PLZA may be enabled or disabled on a per-thread 
basis. However, the COS and RMID association and configuration must be 
the same for all threads in the QOS Domain.

So, PQR_PLZA_ASSOC is a per thread MSR just like PQR_ASSOC.

Privilege-Level Zero Association (PLZA) allows the user to specify a COS 
and/or RMID associated with execution in Privilege-Level Zero. When 
enabled on a HW thread, when that thread enters Privilige-Level Zero, 
transactions associated with that thread will be associated with the 
PLZA COS and/or RMID. Otherwise, the HW thread will be associated with 
the COS and RMID identified by  PQR_ASSOC.

More below.

>
> Consider some of the scenarios:
>
> resctrl mount with default state:
>
> 	# cat info/kernel_mode
> 	[inherit_ctrl_and_mon]
> 	global_assign_ctrl_inherit_mon
> 	global_assign_ctrl_assign_mon
> 	# ls info/kernel_mode_assignment
> 	ls: cannot access 'info/kernel_mode_assignment': No such file or directory
>
> enable global_assign_ctrl_assign_mon mode:
> 	# echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>
> Expectation here is that when user space sets this mode as above then resctrl would
> in turn program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.closid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>
> I do not see why it is necessary to maintain any per-CPU or per-task state or needing
> to touch the context switch code. Since PLZA only supports global could it not
> just set MSR_IA32_PQR_PLZA_ASSOC on all online CPUs and be done with it?
> Only caveat is that if a CPU is offline then this setting needs to be stashed
> so that MSR_IA32_PQR_PLZA_ASSOC can be set when new CPU comes online.
>
> The way that rdtgroup_config_kmode() introduced in patch #11 assumes it is dealing
> with RDT_RESOURCE_L3 and traverses the resource domain list and resource group
> CPU mask seems unnecessary to me as well as error prone since the system may only
> have, for example, RDT_RESOURCE_MBA enabled or even just monitoring. Why not just set
> MSR_IA32_PQR_PLZA_ASSOC on all CPUs and be done?
>
> To continue the scenarios ...
>
> After user's setting above related files read:
> 	# cat info/kernel_mode
> 	inherit_ctrl_and_mon
> 	global_assign_ctrl_inherit_mon
> 	[global_assign_ctrl_assign_mon]
> 	# cat info/kernel_mode_assignment
> 	//
>
> Modify group used by global_assign_ctrl_assign_mon mode:
> 	# echo 'ctrl1/mon1/' > info/kernel_mode_assignment
>
> Expectation here is that when user space sets this then resctrl would
> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid=<rmid of mon1>
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.closid=<closid of ctrl1>
> 	MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.plza_en=1


This works correctly when PLZA associations are defined by per CPU. For 
example, lets assume that *ctrl1* is assigned *CLOSID 1*.

In this scenario, every task in the system running on a any CPU will use 
the limits associated with *CLOSID 1* whenever it enters Privilege-Level 
Zero, because the CPU's *PQR_PLZA_ASSOC* register has PLZA enabled and 
CLOSID is 1.

Now consider task-based association:

We have two resctrl groups:

  * *ctrl1 -> CLOSID 1 -> task1.plza = 1   : *User wants PLZA be enabled
    for this task.
  * *ctrl2 -> CLOSID 2 -> task2.plza = 0   : *User wants PLZA
    disabled for this task.

Suppose *task1* is first scheduled on *CPU 0*. This behaves as expected: 
since CPU 0 's *PQR_PLZA_ASSOC* contains *CLOSID 1, plza_en =1*, task1 
will use the limits from CLOSID 1 when it enters Privilege-Level Zero.

However, if *task2* later runs on *CPU 0*, we expect it to use *CLOSID 
2* in both user mode and kernel mode, because user has PLZA disabled for 
this task. But CPU 0 still has *CLOSID 1, **plza_en =1* in its 
PQR_PLZA_ASSOC register.

As a result, task2 will incorrectly run with *CLOSID 1* when entering 
Privilege-Level Zero something we explicitly want to avoid.

At that point, PLZA must be disabled on CPU 0 to prevent the unintended 
association. Hope this explanation makes the issue clear.

Thanks

Babu

>
> Enable global_assign_ctrl_inherit_mon mode:
> 	# echo "global_assign_ctrl_inherit_mon" > info/kernel_mode
>
> Expectation here is that when user space sets this mode then resctrl would
> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid_en=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>
> 	# cat info/kernel_mode
> 	inherit_ctrl_and_mon
> 	[global_assign_ctrl_inherit_mon]
> 	global_assign_ctrl_assign_mon
> 	# cat info/kernel_mode_assignment <==== returns just a ctrl group
> 	/
>
> Modify group used by global_assign_ctrl_inherit_mon mode:
> 	# echo ctrl1 > info/kernel_mode_assignment
>
> Expectation here is that when user space sets this then resctrl would
> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid_en=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid=<closid of ctrl1>
> 	MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
> 	MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>
> 	# cat info/kernel_mode_assignment <==== returns just a ctrl group
> 	ctrl/
>
> Enable inherit_ctrl_and_mon mode:
> 	# echo "inherit_ctrl_and_mon" > info/kernel_mode
>
> Expectation here is that when user space sets this mode then resctrl would
> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.rmid_en=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid=0
> 	MSR_IA32_PQR_PLZA_ASSOC.closid_en=0
> 	MSR_IA32_PQR_PLZA_ASSOC.plza_en=0
>
> At this point info/kernel_mode_assignment is not visible anymore:
>
> 	# ls info/kernel_mode_assignment
> 	ls: cannot access 'info/kernel_mode_assignment': No such file or directory
>
> >From what I understand above exposes and enables full capability of PLZA. All the other
> per-task and per-cpu handling in this series is not something that PLZA can benefit from.
> If this is not the case, what am I missing? Could this series be simplified to just support
> PLZA today? When next hardware with more capability needs to be supported resctrl could be
> enhanced to support it by using the more accurate information about what the hardware is
> capable of.
>
> We also do not really know what use cases users prefer. This may even be sufficient.
>
> Reinette
>

^ permalink raw reply

* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Maxime Ripard @ 2026-03-26 17:02 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Nicolas Frattaroli, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Thomas Zimmermann, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Sandy Huang, Heiko Stübner, Andy Yan,
	Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	Dmitry Baryshkov, Sascha Hauer, Rob Herring, Jonathan Corbet,
	Shuah Khan, kernel, amd-gfx, dri-devel, linux-kernel,
	linux-arm-kernel, linux-rockchip, intel-gfx, intel-xe, linux-doc,
	Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <acQsw3Wi_xVlBZ8d@intel.com>

[-- Attachment #1: Type: text/plain, Size: 5891 bytes --]

On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > +enum drm_connector_color_format {
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > +	 * helpers should pick a suitable color format. All implementations of a
> > > > > > > > +	 * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > +	 * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > +	 * semantics.
> > > > > > > > +	 *
> > > > > > > > +	 * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > +	 * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > +	 * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > +	 * YCbCr 4:2:0.
> > > > > > > > +	 *
> > > > > > > > +	 * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > +	 * format selection picks the first chain of bridge formats that works,
> > > > > > > > +	 * as has already been the case before the introduction of the "color
> > > > > > > > +	 * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > +	 * bus output formats by preference, or agree on a unified auto format
> > > > > > > > +	 * selection logic that's implemented in a common state helper (like
> > > > > > > > +	 * how HDMI does it).
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > +	 * not subsampled)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > +	 * with horizontal subsampling)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > +
> > > > > > > > +	/**
> > > > > > > > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > +	 * with horizontal and vertical subsampling)
> > > > > > > > +	 */
> > > > > > > > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > 
> > > > > > > Seems like this should document what the quantization range
> > > > > > > should be for each format.
> > > > > > > 
> > > > > > 
> > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > enum is more abstract than that, and is there to communicate
> > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > by other properties.
> > > > > > 
> > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > be added; no digital display protocol standard supports it to my
> > > > > > knowledge, and hopefully none ever will.
> > > > > 
> > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > 
> > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > RGB can either be full or limited range depending on the 
> > > > > "Broadcast RGB" property and other related factors.
> > > > 
> > > > So far the HDMI state has both the format and quantization range as
> > > > different fields. I'm not sure we need to document the range in the
> > > > format field, maybe only mention it's not part of the format but has a
> > > > field of its own?
> > > 
> > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > I think the assumption is limited range everywhere.
> > > 
> > > But I'm not really concerned about documenting struct members.
> > > What I'm talking about is the *uapi* docs. Surely userspace
> > > will want to know what the new property actually does so the
> > > uapi needs to be documented properly. And down the line some
> > > new driver might also implement the wrong behaviour if there
> > > is no clear specification.
> > 
> > Ack
> > 
> > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > - YCbCr limited range 
> > > - RGB full range if "Broadcast RGB" property is not present
> > 
> > Isn't it much more complicated than that for HDMI though? My
> > recollection was that any VIC but VIC1 would be limited range, and
> > anything else full range?
> 
> Do we have some driver that implements the CTA-861 CE vs. IT mode
> logic but doesn't expose the "Broadcast RGB" property? I was hoping
> those would always go hand in hand now.

I'm not sure. i915 and the HDMI state helpers handle it properly (I
think?) but it looks like only vc4 registers the Broadcast RGB property
and uses the HDMI state helpers.

And it looks like amdgpu registers Broadcast RGB but doesn't use
drm_default_rgb_quant_range() which seems suspicious?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

^ permalink raw reply

* Re: [PATCH v3] docs: allow long unbroken headings to wrap and prevent overflow
From: Rito Rhymes @ 2026-03-26 17:00 UTC (permalink / raw)
  To: Jonathan Corbet, Rito Rhymes, linux-doc; +Cc: Shuah Khan, linux-kernel, rdunlap
In-Reply-To: <87fr5nefat.fsf@trenco.lwn.net>

I tested the issues in the following environments:

Windows 10 desktop:
- Chrome
- Firefox
- Edge

Kali Linux under WSL2:
- Chromium
- Firefox

Android 16 mobile:
- Chrome
- Firefox
- Edge

I put together a screenshot catalog of my test results in each
environment, the images are hosted in a GitHub issue and referenced
here. Note that on mobile, when horizontal scroll overflow occurred,
I zoomed out to show the full page width exceeding the header width
to demonstrate the broken layout it caused.

The two cases I used were:

Long slash-delimited string:
https://docs.kernel.org/6.15/userspace-api/sysfs-platform_profile.html

Long underscore-delimited string:
https://docs.kernel.org/6.15/userspace-api/gpio/gpio-v2-line-get-values-ioctl.html

Results by environment:

Windows 10

Chrome:
- slashes: horizontal overflow
  https://github.com/user-attachments/assets/2189783c-0328-4079-a1f0-8aead03e4041
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/d351eb45-44bb-4ae3-8e2c-055a929c776b

Firefox:
- slashes: wraps normally
  https://github.com/user-attachments/assets/fe6667d2-1559-4a50-a1e8-feada0dadd3b
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/b7ee8a66-0c41-457d-b699-9b825fb8b5a1

Edge:
- slashes: horizontal overflow
  https://github.com/user-attachments/assets/703c4e0a-ad0c-42a1-bda6-356a2bc9c609
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/08fa2762-22c4-48b7-aaa4-d418bcc83b62

Kali Linux under WSL2

Chromium:
- slashes: horizontal overflow
  https://github.com/user-attachments/assets/5a25a590-fb84-4445-9bb0-baf069498e41
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/dfa38ba2-2c2b-49af-bd39-31852157e82f

Firefox:
- slashes: wraps normally
  https://github.com/user-attachments/assets/cd736918-4f48-48fe-b18d-822dd2e6727b
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/6ff9e12f-f0b6-445b-a372-aebc93e4a951

Android 16 mobile

Chrome:
- slashes: horizontal overflow
  https://github.com/user-attachments/assets/ea05ef9f-b7ba-4d18-8828-fd3ec7a3db49
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/a4d088ee-d385-499d-bc05-03b8e19870ef

Firefox:
- slashes: wraps normally
  https://github.com/user-attachments/assets/84018ee9-8c87-4d00-99dc-c4569bd16e2c
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/c694acae-18d7-48a1-abcd-a2ae208cfa73

Edge:
- slashes: horizontal overflow
  https://github.com/user-attachments/assets/c5f110f8-3ee8-4490-89f4-d8da33e4f37d
- underscores: horizontal overflow
  https://github.com/user-attachments/assets/4486cb0a-53c1-4d5f-bcc6-d61680133a2c

So the pattern I saw was:

- Long slash-delimited strings wrapped normally only in Firefox, on
  all three environments I tested. In every other browser/environment,
  they caused page-wide horizontal scroll overflow.
- Long underscore-delimited strings caused page-wide horizontal
  scroll overflow in every browser/environment I tested.

If it is easier to review the screenshots in one place,
I collected them in this GitHub issue with environment
headings:

https://github.com/ritovision/linux-kernel-docs/issues/2

That issue may also be a convenient place to add your own
screenshots to host and reference here for comparison.

Rito

^ permalink raw reply

* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Daniel Stone @ 2026-03-26 16:40 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Maxime Ripard, Ville Syrjälä, Harry Wentland, Leo Li,
	Rodrigo Siqueira, Alex Deucher, Christian König,
	David Airlie, Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
	linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
	intel-xe, linux-doc, Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <3979783.tdWV9SEqCh@workhorse>

Hi there,

On Thu, 26 Mar 2026 at 12:44, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
> On Wednesday, 25 March 2026 12:03:07 Central European Standard Time Ville Syrjälä wrote:
> > But I'm not really concerned about documenting struct members.
> > What I'm talking about is the *uapi* docs. Surely userspace
> > will want to know what the new property actually does so the
> > uapi needs to be documented properly. And down the line some
> > new driver might also implement the wrong behaviour if there
> > is no clear specification.
> >
> > So I'm thinking (or perhaps hoping) the rule might be something like:
> > - YCbCr limited range
> > - RGB full range if "Broadcast RGB" property is not present
> > - RGB full or limited range based on the "Broadcast RGB" property
> >   if it's present
> >
> > I think the "Broadcast RGB" property itself might also be lacking
> > proper uapi docs, so that may need to be remedied as well.
>
> Alright, so in v12 I'll do the following:
>
> - Add a line to all YCBCR connector formats that specifies they're
>   limited range as long as Broadcast RGB is limited. Whether it's limited
>   range when Broadcast RGB is full is purposefully left undefined.
>   In the future, we can expand this to state they're limited range by
>   default unless some other property is set. If we're not re-using
>   Broadcast RGB for that, this will work out fine, because users who
>   don't know about the eventual new property won't have this behaviour
>   changed. If we do re-use "Broadcast RGB" for that, then only users
>   relying on things we explicitly left undefined will get surprise
>   full range YCBCR.
> - Add a line to the RGB connector format that specifies its range
>   depends on the "Broadcast RGB" property
>
> This is a bit of a mess, because it's entirely reasonable that a
> future YCBCR range property would want to default to full range
> so that users get the most color out of their monitors. But with
> this description of the connector color formats, we can't do that.
>
> If there are alternate suggestions, I'm open for them. We can't
> really rename "Broadcast RGB" but if I had a time machine, that'd
> be my first choice.

'Broadcast RGB' isn't what you want even if it could handle YUV, since
it also sets up colour transforms to modify the data ... so we need a
separate, orthogonal, property which only affects the HDMI infoframe,
rather than applying any transforms.

Cheers,
Daniel

^ permalink raw reply

* [PATCH 1/1] docs: kdoc_diff: add a helper tool to help checking kdoc regressions
From: Mauro Carvalho Chehab @ 2026-03-26 16:22 UTC (permalink / raw)
  To: Jonathan Corbet, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, linux-kernel, Mauro Carvalho Chehab,
	Shuah Khan
In-Reply-To: <cover.1774541999.git.mchehab+huawei@kernel.org>

Checking for regressions at kernel-doc can be hard. Add a helper
tool to make such task easier.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 tools/docs/kdoc_diff | 504 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 504 insertions(+)
 create mode 100755 tools/docs/kdoc_diff

diff --git a/tools/docs/kdoc_diff b/tools/docs/kdoc_diff
new file mode 100755
index 000000000000..5edd9b46a825
--- /dev/null
+++ b/tools/docs/kdoc_diff
@@ -0,0 +1,504 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2026: Mauro Carvalho Chehab <mchehab@kernel.org>.
+#
+# pylint: disable=R0903,R0912,R0913,R0914,R0915,R0917
+
+"""
+docdiff - Check differences between kernel‑doc output between two different
+commits.
+
+Examples
+--------
+
+Compare the kernel‑doc output between the last two 5.15 releases::
+
+    $ kdoc_diff v6.18..v6.19
+
+Both outputs are cached
+
+Force a complete documentation scan and clean any previous cache from
+6.19 to the current HEAD::
+
+    $ kdoc_diff 6.19.. --full --clean
+
+Check differences only on a single driver since origin/main::
+
+    $ kdoc_diff origin/main drivers/media
+
+Generate an YAML file and use it to check for regressions::
+
+    $ kdoc_diff HEAD~ drivers/media --regression
+
+
+"""
+
+import os
+import sys
+import argparse
+import subprocess
+import shutil
+import re
+import signal
+
+from glob import iglob
+
+
+SRC_DIR = os.path.dirname(os.path.realpath(__file__))
+WORK_DIR = os.path.abspath(os.path.join(SRC_DIR, "../.."))
+
+KDOC_BINARY = os.path.join(SRC_DIR, "kernel-doc")
+KDOC_PARSER_TEST = os.path.join(WORK_DIR, "tools/unittests/test_kdoc_parser.py")
+
+CACHE_DIR = ".doc_diff_cache"
+
+DIR_NAME = {
+    "full": os.path.join(CACHE_DIR, "full"),
+    "partial": os.path.join(CACHE_DIR, "partial"),
+    "no-cache": os.path.join(CACHE_DIR, "no_cache"),
+    "tmp": os.path.join(CACHE_DIR, "__tmp__"),
+}
+
+class GitHelper:
+    """Handles all Git operations"""
+
+    def __init__(self, work_dir=None):
+        self.work_dir = work_dir
+
+    def is_inside_repository(self):
+        """Check if we're inside a Git repository"""
+        try:
+            output = subprocess.check_output(["git", "rev-parse",
+                                              "--is-inside-work-tree"],
+                                             cwd=self.work_dir,
+                                             stderr=subprocess.STDOUT,
+                                             universal_newlines=True)
+
+            return output.strip() == "true"
+        except subprocess.CalledProcessError:
+            return False
+
+    def is_valid_commit(self, commit_hash):
+        """
+        Validate that a ref (branch, tag, commit hash, etc.) can be
+        resolved to a commit.
+        """
+        try:
+            subprocess.check_output(["git", "rev-parse", commit_hash],
+                                    cwd=self.work_dir,
+                                    stderr=subprocess.STDOUT)
+            return True
+        except subprocess.CalledProcessError:
+            return False
+
+    def get_short_hash(self, commit_hash):
+        """Get short commit hash"""
+        try:
+            return subprocess.check_output(["git", "rev-parse", "--short",
+                                            commit_hash],
+                                           cwd=self.work_dir,
+                                           stderr=subprocess.STDOUT,
+                                           universal_newlines=True).strip()
+        except subprocess.CalledProcessError:
+            return ""
+
+    def has_uncommitted_changes(self):
+        """Check for uncommitted changes"""
+        try:
+            subprocess.check_output(["git", "diff-index",
+                                     "--quiet", "HEAD", "--"],
+                                    cwd=self.work_dir,
+                                    stderr=subprocess.STDOUT)
+            return False
+        except subprocess.CalledProcessError:
+            return True
+
+    def get_current_branch(self):
+        """Get current branch name"""
+        return subprocess.check_output(["git", "branch", "--show-current"],
+                                        cwd=self.work_dir,
+                                        universal_newlines=True).strip()
+
+    def checkout_commit(self, commit_hash, quiet=True):
+        """Checkout a commit safely"""
+        args = ["git", "checkout", "-f"]
+        if quiet:
+            args.append("-q")
+        args.append(commit_hash)
+        try:
+            subprocess.check_output(args, cwd=self.work_dir,
+                                    stderr=subprocess.STDOUT)
+
+            # Double-check if branch actually switched
+            branch = self.get_short_hash("HEAD")
+            if commit_hash != branch:
+                raise RuntimeError(f"Branch changed to '{branch}' instead of '{commit_hash}'")
+
+            return True
+        except subprocess.CalledProcessError as e:
+            print(f"ERROR: Failed to checkout {commit_hash}: {e}",
+                  file=sys.stderr)
+            return False
+
+
+class CacheManager:
+    """Manages persistent cache directories"""
+
+    def __init__(self, work_dir):
+        self.work_dir = work_dir
+
+    def initialize(self):
+        """Create cache directories if they don't exist"""
+        for dir_path in DIR_NAME.values():
+            abs_path = os.path.join(self.work_dir, dir_path)
+            if not os.path.exists(abs_path):
+                os.makedirs(abs_path, exist_ok=True, mode=0o755)
+
+    def get_commit_cache(self, commit_hash, path):
+        """Generate cache path for a commit"""
+        hash_short = GitHelper(self.work_dir).get_short_hash(commit_hash)
+        if not hash_short:
+            hash_short = commit_hash
+
+        return os.path.join(path, hash_short)
+
+class KernelDocRunner:
+    """Runs kernel-doc documentation generator"""
+
+    def __init__(self, work_dir, kdoc_binary):
+        self.work_dir = work_dir
+        self.kdoc_binary = kdoc_binary
+        self.kdoc_files = None
+
+    def find_kdoc_references(self):
+        """Find all files marked with kernel-doc:: directives"""
+        if self.kdoc_files:
+            print("Using cached Kdoc refs")
+            return self.kdoc_files
+
+        print("Finding kernel-doc entries in Documentation...")
+
+        files = os.path.join(self.work_dir, 'Documentation/**/*.rst')
+        pattern = re.compile(r"^\.\.\s+kernel-doc::\s*(\S+)")
+        kdoc_files = set()
+
+        for file_path in iglob(files, recursive=True):
+            try:
+                with open(file_path, 'r', encoding='utf-8') as fp:
+                    for line in fp:
+                        match = pattern.match(line.strip())
+                        if match:
+                            kdoc_files.add(match.group(1))
+
+            except OSError:
+                continue
+
+        self.kdoc_files = list(kdoc_files)
+
+        return self.kdoc_files
+
+    def gen_yaml(self, yaml_file, kdoc_files):
+        """Runs kernel-doc to generate a yaml file with man and rst."""
+        cmd = [self.kdoc_binary, "--man", "--rst", "--yaml", yaml_file]
+        cmd += kdoc_files
+
+        try:
+            subprocess.check_call(cmd, cwd=self.work_dir,
+                                  stdout=subprocess.DEVNULL,
+                                  stderr=subprocess.DEVNULL)
+        except subprocess.CalledProcessError:
+            return False
+
+        return True
+
+    def run_unittest(self, yaml_file):
+        """Run unit tests with the generated yaml file"""
+        try:
+            subprocess.check_call([KDOC_PARSER_TEST, "--yaml", yaml_file],
+                                  cwd=self.work_dir)
+        except subprocess.CalledProcessError:
+            return False
+
+        return True
+
+    def normal_run(self, tmp_dir, output_dir, kdoc_files):
+        """Generate man, rst and errors, storing them at tmp_dir."""
+        os.makedirs(tmp_dir, exist_ok=True)
+
+        try:
+            with open(os.path.join(tmp_dir, "man.log"), "w", encoding="utf-8") as out:
+                subprocess.check_call([self.kdoc_binary, "--man"] + kdoc_files,
+                                      cwd=self.work_dir,
+                                      stdout=out, stderr=subprocess.DEVNULL)
+
+            with open(os.path.join(tmp_dir, "rst.log"), "w", encoding="utf-8") as out:
+                with open(os.path.join(tmp_dir, "err.log"), "w", encoding="utf-8") as err:
+                    subprocess.check_call([self.kdoc_binary, "--rst"] + kdoc_files,
+                                          cwd=self.work_dir,
+                                          stdout=out, stderr=err)
+        except subprocess.CalledProcessError:
+            return False
+
+        if output_dir:
+            os.replace(tmp_dir, output_dir)
+
+        return True
+
+    def run(self, commit_hash, tmp_dir, output_dir, kdoc_files, is_regression,
+            is_end):
+        """Run kernel-doc on its several ways"""
+        if not kdoc_files:
+            raise RuntimeError("No kernel-doc references found")
+
+        git_helper = GitHelper(self.work_dir)
+        if not git_helper.checkout_commit(commit_hash, quiet=True):
+            raise RuntimeError(f"ERROR: can't checkout commit {commit_hash}")
+
+        print(f"Processing {commit_hash}...")
+
+        if not is_regression:
+            return self.normal_run(tmp_dir, output_dir, kdoc_files)
+
+        yaml_file = os.path.join(tmp_dir, "out.yaml")
+
+        if not is_end:
+            return self.gen_yaml(yaml_file, kdoc_files)
+
+        return self.run_unittest(yaml_file)
+
+class DiffManager:
+    """Compare documentation output directories with an external diff."""
+    def __init__(self, diff_tool="diff", diff_args=None):
+        self.diff_tool = diff_tool
+        # default: unified, no context, ignore whitespace changes
+        self.diff_args = diff_args or ["-u0", "-w"]
+
+    def diff_directories(self, dir1, dir2):
+        """Compare two directories using an external diff."""
+        print(f"\nDiffing {dir1} and {dir2}:")
+
+        dir1_files = set()
+        dir2_files = set()
+        has_diff = False
+
+        for root, _, files in os.walk(dir1):
+            for file in files:
+                dir1_files.add(os.path.relpath(os.path.join(root, file), dir1))
+        for root, _, files in os.walk(dir2):
+            for file in files:
+                dir2_files.add(os.path.relpath(os.path.join(root, file), dir2))
+
+        common_files = sorted(dir1_files & dir2_files)
+        for file in common_files:
+            f1 = os.path.join(dir1, file)
+            f2 = os.path.join(dir2, file)
+
+            cmd = [self.diff_tool] + self.diff_args + [f1, f2]
+            try:
+                result = subprocess.run(
+                    cmd, capture_output=True, text=True, check=False
+                )
+                if result.stdout:
+                    has_diff = True
+                    print(f"\n{file}")
+                    print(result.stdout, end="")
+            except FileNotFoundError:
+                print(f"ERROR: {self.diff_tool} not found")
+                sys.exit(1)
+
+        # Show files that exist only in one directory
+        only_in_dir1 = dir1_files - dir2_files
+        only_in_dir2 = dir2_files - dir1_files
+        if only_in_dir1 or only_in_dir2:
+            has_diff = True
+            print("\nDifferential files:")
+            for f in sorted(only_in_dir1):
+                print(f"  - {f} (only in {dir1})")
+            for f in sorted(only_in_dir2):
+                print(f"  + {f} (only in {dir2})")
+
+        if not has_diff:
+            print("\nNo differences between those two commits")
+
+
+class SignalHandler():
+    """Signal handler class."""
+
+    def restore(self, force_exit=False):
+        """Restore original HEAD state."""
+        if self.restored:
+            return
+
+        print(f"Restoring original branch: {self.original_head}")
+        try:
+            subprocess.check_call(
+                ["git", "checkout", "-f", self.original_head],
+                cwd=self.git_helper.work_dir,
+                stderr=subprocess.STDOUT,
+            )
+        except subprocess.CalledProcessError as e:
+            print(f"Failed to restore: {e}", file=sys.stderr)
+
+        for sig, handler in self.old_handler.items():
+            signal.signal(sig, handler)
+
+        self.restored = True
+
+        if force_exit:
+            sys.exit(1)
+
+    def signal_handler(self, sig, _):
+        """Handle interrupt signals."""
+        print(f"\nSignal {sig} received. Restoring original state...")
+
+        self.restore(force_exit=True)
+
+    def __enter__(self):
+        """Allow using it via with command."""
+        for sig in [signal.SIGINT, signal.SIGTERM]:
+            self.old_handler[sig] = signal.getsignal(sig)
+            signal.signal(sig, self.signal_handler)
+
+        return self
+
+    def __exit__(self, *args):
+        """Restore signals at the end of with block."""
+        self.restore()
+
+    def __init__(self, git_helper, original_head):
+        self.git_helper = git_helper
+        self.original_head = original_head
+        self.old_handler = {}
+        self.restored = False
+
+def parse_commit_range(value):
+    """Handle a commit range."""
+    if ".." not in value:
+        begin = value
+        end = "HEAD"
+    else:
+        begin, _, end = value.partition("..")
+        if not end:
+            end = "HEAD"
+
+    if not begin:
+        raise argparse.ArgumentTypeError("Need a commit begginning")
+
+
+    print(f"Range: {begin} to {end}")
+
+    return begin, end
+
+
+def main():
+    """Main code"""
+    parser = argparse.ArgumentParser(description="Compare kernel documentation between commits")
+    parser.add_argument("commits", type=parse_commit_range,
+                        help="commit range like old..new")
+    parser.add_argument("files", nargs="*",
+                        help="files to process – if supplied the --full flag is ignored")
+
+    parser.add_argument("--full", "-f", action="store_true",
+                        help="Force a full scan of Documentation/*")
+
+    parser.add_argument("--regression", "-r", action="store_true",
+                        help="Use YAML format to check for regressions")
+
+    parser.add_argument("--work-dir", "-w", default=WORK_DIR,
+                        help="work dir (default: %(default)s)")
+
+    parser.add_argument("--clean", "-c", action="store_true",
+                        help="Clean caches")
+
+    args = parser.parse_args()
+
+    if args.files and args.full:
+        raise argparse.ArgumentError(args.full,
+                                     "cannot combine '--full' with an explicit file list")
+
+    work_dir = os.path.abspath(args.work_dir)
+
+    # Initialize cache
+    cache = CacheManager(work_dir)
+    cache.initialize()
+
+    # Validate git repository
+    git_helper = GitHelper(work_dir)
+    if not git_helper.is_inside_repository():
+        raise RuntimeError("Must run inside Git repository")
+
+    old_commit, new_commit = args.commits
+
+    old_commit = git_helper.get_short_hash(old_commit)
+    new_commit = git_helper.get_short_hash(new_commit)
+
+    # Validate commits
+    for commit in [old_commit, new_commit]:
+        if not git_helper.is_valid_commit(commit):
+            raise RuntimeError(f"Commit '{commit}' does not exist")
+
+    # Check for uncommitted changes
+    if git_helper.has_uncommitted_changes():
+        raise RuntimeError("Uncommitted changes present. Commit or stash first.")
+
+    runner = KernelDocRunner(git_helper.work_dir, KDOC_BINARY)
+
+    # Get files to be parsed
+    cache_msg = " (results will be cached)"
+    if args.full:
+        kdoc_files = ["."]
+        diff_type = "full"
+        print(f"Parsing all files at {work_dir}")
+    if not args.files:
+        diff_type = "partial"
+        kdoc_files = runner.find_kdoc_references()
+        print(f"Parsing files with kernel-doc markups at {work_dir}/Documentation")
+    else:
+        diff_type = "no-cache"
+        cache_msg = ""
+        kdoc_files = args.files
+
+    if args.regression:
+        cache_msg = ""
+
+    out_path = DIR_NAME[diff_type]
+    print(f"Output will be stored at: {out_path}{cache_msg}")
+
+    # Just in case - should never happen in practice
+    if not kdoc_files:
+        raise argparse.ArgumentError(args.files,
+                                        "No kernel-doc references found")
+
+    original_head = git_helper.get_current_branch()
+    tmp_dir = DIR_NAME["tmp"]
+
+    old_cache = cache.get_commit_cache(old_commit, out_path)
+    new_cache = cache.get_commit_cache(new_commit, out_path)
+
+    with SignalHandler(git_helper, original_head):
+        if args.clean or diff_type == "no-cache":
+            for cache_dir in [old_cache, new_cache]:
+                if cache_dir and os.path.exists(cache_dir):
+                    shutil.rmtree(cache_dir)
+
+        if args.regression or not os.path.exists(old_cache):
+            old_success = runner.run(old_commit, tmp_dir, old_cache, kdoc_files,
+                                    args.regression, False)
+        else:
+            old_success = True
+
+        if args.regression or not os.path.exists(new_cache):
+            new_success = runner.run(new_commit, tmp_dir, new_cache, kdoc_files,
+                                    args.regression, True)
+        else:
+            new_success = True
+
+    if not (old_success and new_success):
+        raise RuntimeError("Failed to generate documentation")
+
+    if not args.regression:
+        diff_manager = DiffManager()
+        diff_manager.diff_directories(old_cache, new_cache)
+
+if __name__ == "__main__":
+    main()
-- 
2.52.0


^ permalink raw reply related

* [PATCH 0/1] Add a script to check for kernel-doc regressions
From: Mauro Carvalho Chehab @ 2026-03-26 16:21 UTC (permalink / raw)
  To: Jonathan Corbet, Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, linux-doc, linux-kernel, Shuah Khan

Hi Jon,

I've using this script internally to check for regressions and
changes with kernel-doc, specially those related to the new
CTokenizer code:

	$ tools/docs/kdoc_diff --help
	usage: kdoc_diff [-h] [--full] [--regression] [--work-dir WORK_DIR] [--clean] commits [files ...]

	Compare kernel documentation between commits

	positional arguments:
	  commits               commit range like old..new
	  files                 files to process – if supplied the --full flag is ignored

	options:
	  -h, --help            show this help message and exit
	  --full, -f            Force a full scan of Documentation/*
	  --regression, -r      Use YAML format to check for regressions
	  --work-dir, -w WORK_DIR
	                        work dir (default: /new_devel/docs)
	  --clean, -c           Clean caches

I did today a cleanup, to be able to submit it, as I think it could
be helpful to you and others as well, as it automates the diff check
between two commits.

It has two modes of work:

1. It generates 3 files: err.log, man.log, rst.log and does
   a diff between old/new commit.

   On this mode, it sorts err.log and remove duplicated messages,
   so it relaxes a little bit the diff comparision, if a minor
   change affects its error output.

2. It uses yaml to run regressions test.

   The regressions mode is nice when no regressions are expected. It
   uses the tools/unittest/test_kdoc_parser, which is somewhat relaxed
   with regards to trivial changes like whitespaces.

The tested files can either be:

a. Partial: only files explicitly included via  kernel-doc:: markups
   inside Documentation;

b. Full: includes files with broken kernel-doc markups that are all
   spread inside Kernel tree;

c. A list of files or directories.

To prevent losing anything, before running, it checks if the tree
is not dirty. While running, it does git checkout -f, and, at the
end, it returns to the current branch.

There's a logic there which catches signals to avoid troubles on
errors/exit/ctrl-c. At least on my tests, it worked fine even
on python errors inside the script. Yet, in case of troubles,
one could use git reflog.

---

With regards with tools usage:

The v0.1 skeleton was originally written via LLM (gpt-oss), but the
code was almost entirely rewritten by hand. It was also checked with
pylint and I re-checked it again with another LLM (nemotron2-cascade),
all executed on my local machine, without Internet access enabled
on ollama.

LLM prototyping was interesting to have a quick start code but, as
expected, LLM output it not anything better than any other traditional
auto-complete/auto-generated code method: the produced code was
complex, with lots of caveats and hidden issues. Yet, using LLM for
some specific tasks (like for instance to write a signal handler) is
usually faster than googling at the Internet. Also, it helps to review
the logic, as it can point to some problems, but its output requires
one with enough knowledge to discard bad code and ignore AI hallucinations.

Mauro Carvalho Chehab (1):
  docs: kdoc_diff: add a helper tool to help checking kdoc regressions

 tools/docs/kdoc_diff | 504 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 504 insertions(+)
 create mode 100755 tools/docs/kdoc_diff

-- 
2.52.0


^ permalink raw reply

* Re: [PATCH] docs: kernel-parameters: fix architecture alignment for pt, nopt, and nobypass
From: Randy Dunlap @ 2026-03-26 16:07 UTC (permalink / raw)
  To: lirongqing, Jonathan Corbet, Shuah Khan, Andrew Morton,
	Borislav Petkov, Peter Zijlstra, Feng Tang, Pawan Gupta,
	Dapeng Mi, Kees Cook, Marco Elver, Paul E . McKenney, Askar Safin,
	Bjorn Helgaas, Sohil Mehta, linux-doc, linux-kernel
In-Reply-To: <20260326074658.1899-1-lirongqing@baidu.com>

Hi,

On 3/26/26 12:46 AM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> Commit ab0e7f20768a ("Documentation: Merge x86-specific boot options doc
> into kernel-parameters.txt") introduced a formatting regression where
> architecture tags were placed on separate lines with broken indentation.
> This caused the 'nopt' [X86] parameter to appear as if it belonged to
> the [PPC/POWERNV] section.
> 
> Fix the formatting by placing the architecture tags on the same line as
> their respective parameters ('pt', 'nopt', and 'nobypass') and restoring
> proper indentation.
> 
> Fixes: ab0e7f20768a ("Documentation: Merge x86-specific boot options doc into kernel-parameters.txt")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 03a5506..dc1c5bd 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2615,12 +2615,9 @@ Kernel parameters
>  			Intel machines). This can be used to prevent the usage
>  			of an available hardware IOMMU.
>  
> -			[X86]
> -		pt
> -			[X86]
> -		nopt
> -			[PPC/POWERNV]
> -		nobypass
> +		pt  	[X86]
> +		nopt	[X86]
> +		nobypass	[PPC/POWERNV]
>  			Disable IOMMU bypass, using IOMMU for PCI devices.
>  
>  		[X86]

This looks good as far as it goes, but there are still some problems IMO.

These are all iommu= options, but iommu says that it's for [X86,EARLY].
No PPC/POWERNV mentioned there.

Then immediately following nobypass, there is this:
		[X86]
		AMD Gart HW IOMMU-specific options:

which is also in questionable format. The [X86] isn't needed at all IMO,
or if it's desirable, those 2 lines should be on one line.

Anyway, for this patch:
Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

-- 
~Randy

^ permalink raw reply

* Re: [PATCH v3 09/10] dt-bindings: firmware: add arm,ras-cper
From: Rob Herring @ 2026-03-26 15:24 UTC (permalink / raw)
  To: Ahmed Tiba
  Cc: linux-acpi, devicetree, linux-cxl, Michael.Zhao2,
	linux-arm-kernel, Dmitry.Lamerov, rafael, conor, will, bp,
	catalin.marinas, krzk+dt, linux-doc, mchehab+huawei, tony.luck
In-Reply-To: <20260318-topics-ahmtib01-ras_ffh_arm_internal_review-v3-9-48e6a1c249ef@arm.com>

On Wed, Mar 18, 2026 at 08:48:06PM +0000, Ahmed Tiba wrote:
> Describe the DeviceTree node that exposes the Arm firmware-first
> CPER provider and hook the file into MAINTAINERS so the
> binding has an owner.
> 
> Signed-off-by: Ahmed Tiba <ahmed.tiba@arm.com>
> ---
>  .../devicetree/bindings/firmware/arm,ras-cper.yaml | 71 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,ras-cper.yaml b/Documentation/devicetree/bindings/firmware/arm,ras-cper.yaml
> new file mode 100644
> index 000000000000..bd93cfb8d222
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/arm,ras-cper.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/arm,ras-cper.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Arm RAS CPER provider
> +
> +maintainers:
> +  - Ahmed Tiba <ahmed.tiba@arm.com>
> +
> +description: |
> +  Arm Reliability, Availability and Serviceability (RAS) firmware can expose
> +  a firmware-first CPER error source directly via DeviceTree. Firmware
> +  provides the CPER Generic Error Status block and notifies the OS through
> +  an interrupt.
> +
> +properties:
> +  compatible:
> +    const: arm,ras-cper
> +
> +  reg:
> +    minItems: 1
> +    items:
> +      - description:
> +          CPER Generic Error Status block exposed by firmware
> +      - description:
> +          Optional 32- or 64-bit doorbell register used on platforms
> +          where firmware needs an explicit "ack" handshake before overwriting
> +          the CPER buffer. Firmware watches bit 0 and expects the OS to set it
> +          once the current status block has been consumed.
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Interrupt used to signal that a new status record is ready.
> +
> +  memory-region:
> +    $ref: /schemas/types.yaml#/definitions/phandle

memory-region already has a defined type. You just need to define how 
many entries (maxItems: 1).

> +    description:
> +      Optional phandle to the reserved-memory entry that backs the status
> +      buffer so firmware and the OS use the same carved-out region.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    reserved-memory {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +      ras_cper_buffer: cper@fe800000 {
> +        reg = <0x0 0xfe800000 0x0 0x1000>;
> +        no-map;
> +      };
> +    };
> +
> +    error-handler@fe800000 {
> +      compatible = "arm,ras-cper";
> +      reg = <0xfe800000 0x1000>,

Wait! Why is the reserved address here? There's 2 problems with that. 
There shouldn't be same address in 2 places in the DT. The 2nd is 
reserved memory should only be regions within DRAM (or whatever is 
system memory).

Rob

^ permalink raw reply

* Re: [PATCH] checkpatch: allow correctly handle full files on stdin
From: Dmitry Torokhov @ 2026-03-26 14:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Dwaipayan Ray, Lukas Bulwahn, Andy Whitcroft, Jonathan Corbet,
	Shuah Khan, workflows, linux-doc, linux-kernel
In-Reply-To: <bb47800754aa3279e88c9d88c380bcfe6263fb2d.camel@perches.com>

On Thu, Mar 26, 2026 at 01:46:49AM -0700, Joe Perches wrote:
> On Wed, 2026-03-25 at 23:20 -0700, Dmitry Torokhov wrote:
> > checkpatch does not handle full files well when they are passed on
> > stdin, because it does not know how to treat the text, and whether it is
> > a C file, or a DTS file, or something else, and so it assumes that when
> > it works with stdin it should be a unified diff. For full files it
> > expects to have a file name as an argument and read the contents from
> > disk. Unfortunately this does not well when trying to use checkpatch as
> > an online linter and feed it contents of an editor buffer that have not
> > made it to the disk yet.
> 
> Why is this useful?
> Why not save the buffer and then feed the file?

Because when I am editing a file I am not saving it all that often. I
want to have buffer diagnostic updated when I leave insert mode in vim.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH V9 3/8] dax: add fsdev.c driver for fs-dax on character dax
From: John Groves @ 2026-03-26 14:33 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Jonathan Cameron, John Groves, Miklos Szeredi, Dan Williams,
	Bernd Schubert, Alison Schofield, John Groves, Jonathan Corbet,
	Shuah Khan, Vishal Verma, Dave Jiang, Matthew Wilcox, Jan Kara,
	Alexander Viro, David Hildenbrand, Christian Brauner,
	Darrick J . Wong, Randy Dunlap, Jeff Layton, Amir Goldstein,
	Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
	Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
	Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
	Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
	linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org
In-Reply-To: <69c407903b54c_130d6e1007a@iweiny-mobl.notmuch>

On 26/03/25 11:04AM, Ira Weiny wrote:
> John Groves wrote:
> > On 26/03/24 02:39PM, Jonathan Cameron wrote:
> > > On Tue, 24 Mar 2026 00:38:31 +0000
> > > John Groves <john@jagalactic.com> wrote:
> > > 
> > > > From: John Groves <john@groves.net>
> > > > 
> > > > The new fsdev driver provides pages/folios initialized compatibly with
> > > > fsdax - normal rather than devdax-style refcounting, and starting out
> > > > with order-0 folios.
> > > > 
> > > > When fsdev binds to a daxdev, it is usually (always?) switching from the
> > > > devdax mode (device.c), which pre-initializes compound folios according
> > > > to its alignment. Fsdev uses fsdev_clear_folio_state() to switch the
> > > > folios into a fsdax-compatible state.
> > > > 
> > > > A side effect of this is that raw mmap doesn't (can't?) work on an fsdev
> > > > dax instance. Accordingly, The fsdev driver does not provide raw mmap -
> > > > devices must be put in 'devdax' mode (drivers/dax/device.c) to get raw
> > > > mmap capability.
> > > > 
> > > > In this commit is just the framework, which remaps pages/folios compatibly
> > > > with fsdax.
> > > > 
> > > > Enabling dax changes:
> > > > 
> > > > - bus.h: add DAXDRV_FSDEV_TYPE driver type
> > > > - bus.c: allow DAXDRV_FSDEV_TYPE drivers to bind to daxdevs
> > > > - dax.h: prototype inode_dax(), which fsdev needs
> > > > 
> > > > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > > > Suggested-by: Gregory Price <gourry@gourry.net>
> > > > Signed-off-by: John Groves <john@groves.net>
> > > 
> > > I was kind of thinking you'd go with a hidden KCONFIG option with default
> > > magic to do the same build condition to you had in the Makefil, but one the
> > > user can opt in or out for is also fine.
> > > 
> > > Comments on that below. Meh, I think this is better anyway :)
> > > 
> > > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > > 
> > > 
> > > 
> > > > diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
> > > > index d656e4c0eb84..7051b70980d5 100644
> > > > --- a/drivers/dax/Kconfig
> > > > +++ b/drivers/dax/Kconfig
> > > > @@ -61,6 +61,17 @@ config DEV_DAX_HMEM_DEVICES
> > > >  	depends on DEV_DAX_HMEM && DAX
> > > >  	def_bool y
> > > >  
> > > > +config DEV_DAX_FSDEV
> > > > +	tristate "FSDEV DAX: fs-dax compatible devdax driver"
> > > > +	depends on DEV_DAX && FS_DAX
> > > > +	help
> > > > +	  Support fs-dax access to DAX devices via a character device
> > > > +	  interface. Unlike device_dax (which pre-initializes compound folios
> > > > +	  based on device alignment), this driver leaves folios at order-0 so
> > > > +	  that fs-dax filesystems can manage folio order dynamically.
> > > > +
> > > > +	  Say M if unsure.
> > > Fine like this, but if you wanted to hide it in interests of not
> > > confusing users...
> > > 
> > > config DEV_DAX_FSDEV
> > > 	tristate
> > > 	depends on DEV_DAX && FS_DAX
> > > 	default DEV_DAX
> > 
> > I like this better. I see no reason not to default to including fsdev.
> > It does nothing other than frustrating famfs users if it's off - since
> > building it still has no effect unless you put a daxdev in famfs mode.
> > 
> > Ira, it's kinda in your hands at the moment. Do you feel like making this
> > change?
> 
> I don't mind making this change.  But we have to deal with the breakage to
> current device dax users.
> 
> https://lore.kernel.org/all/69c36921255b6_e9d8d1009b@iweiny-mobl.notmuch/
> 
> What am I missing?
> 
> Ira

OK, I can reproduce that failure with kernel 7.0.0-rc5 and 
straight ndctl v84. So it's not famfs.

I also studied the verbose logs trying to figure out if famfs
could cause it (while running a famfs kernel and ndctl), but
I don't see it.

Then I tried non-famfs kernel and ndctl and it's the same with
or without famfs kernel and famfs ndctl.

John


^ permalink raw reply

* Re: [PATCH v3 4/5] KVM: arm64: Enable HDBSS support and handle HDBSSF events
From: Leonardo Bras @ 2026-03-26 14:31 UTC (permalink / raw)
  To: Tian Zheng
  Cc: Leonardo Bras, maz, oupton, catalin.marinas, corbet, pbonzini,
	will, yuzenghui, wangzhou1, liuyonglong, Jonathan.Cameron,
	yezhenyu2, linuxarm, joey.gouly, kvmarm, kvm, linux-arm-kernel,
	linux-doc, linux-kernel, skhan, suzuki.poulose
In-Reply-To: <acQj5grOdZT8LUGp@devkitleo>

On Wed, Mar 25, 2026 at 06:05:26PM +0000, Leonardo Bras wrote:
> Hello Tian,
> 
> I am currently working on HACDBS enablement(which will be rebased on top of 
> this patchset) and due to the fact HACDBS and HDBSS are kind of 
> complementary I will sometimes come with some questions for issues I have 
> faced myself on that part. :)
> 
> (see below)
> 
> On Wed, Feb 25, 2026 at 12:04:20PM +0800, Tian Zheng wrote:
> > From: eillon <yezhenyu2@huawei.com>
> > 
> > HDBSS is enabled via an ioctl from userspace (e.g. QEMU) at the start of
> > migration. This feature is only supported in VHE mode.
> > 
> > Initially, S2 PTEs doesn't contain the DBM attribute. During migration,
> > write faults are handled by user_mem_abort, which relaxes permissions
> > and adds the DBM bit when HDBSS is active. Once DBM is set, subsequent
> > writes no longer trap, as the hardware automatically transitions the page
> > from writable-clean to writable-dirty.
> > 
> > KVM does not scan S2 page tables to consume DBM. Instead, when HDBSS is
> > enabled, the hardware observes the clean->dirty transition and records
> > the corresponding page into the HDBSS buffer.
> > 
> > During sync_dirty_log, KVM kicks all vCPUs to force VM-Exit, ensuring
> > that check_vcpu_requests flushes the HDBSS buffer and propagates the
> > accumulated dirty information into the userspace-visible dirty bitmap.
> > 
> > Add fault handling for HDBSS including buffer full, external abort, and
> > general protection fault (GPF).
> > 
> > Signed-off-by: eillon <yezhenyu2@huawei.com>
> > Signed-off-by: Tian Zheng <zhengtian10@huawei.com>
> > ---
> >  arch/arm64/include/asm/esr.h      |   5 ++
> >  arch/arm64/include/asm/kvm_host.h |  17 +++++
> >  arch/arm64/include/asm/kvm_mmu.h  |   1 +
> >  arch/arm64/include/asm/sysreg.h   |  11 ++++
> >  arch/arm64/kvm/arm.c              | 102 ++++++++++++++++++++++++++++++
> >  arch/arm64/kvm/hyp/vhe/switch.c   |  19 ++++++
> >  arch/arm64/kvm/mmu.c              |  70 ++++++++++++++++++++
> >  arch/arm64/kvm/reset.c            |   3 +
> >  8 files changed, 228 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> > index 81c17320a588..2e6b679b5908 100644
> > --- a/arch/arm64/include/asm/esr.h
> > +++ b/arch/arm64/include/asm/esr.h
> > @@ -437,6 +437,11 @@
> >  #ifndef __ASSEMBLER__
> >  #include <asm/types.h>
> > 
> > +static inline bool esr_iss2_is_hdbssf(unsigned long esr)
> > +{
> > +	return ESR_ELx_ISS2(esr) & ESR_ELx_HDBSSF;
> > +}
> > +
> >  static inline unsigned long esr_brk_comment(unsigned long esr)
> >  {
> >  	return esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 5d5a3bbdb95e..57ee6b53e061 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -55,12 +55,17 @@
> >  #define KVM_REQ_GUEST_HYP_IRQ_PENDING	KVM_ARCH_REQ(9)
> >  #define KVM_REQ_MAP_L1_VNCR_EL2		KVM_ARCH_REQ(10)
> >  #define KVM_REQ_VGIC_PROCESS_UPDATE	KVM_ARCH_REQ(11)
> > +#define KVM_REQ_FLUSH_HDBSS			KVM_ARCH_REQ(12)
> > 
> >  #define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
> >  				     KVM_DIRTY_LOG_INITIALLY_SET)
> > 
> >  #define KVM_HAVE_MMU_RWLOCK
> > 
> > +/* HDBSS entry field definitions */
> > +#define HDBSS_ENTRY_VALID BIT(0)
> > +#define HDBSS_ENTRY_IPA GENMASK_ULL(55, 12)
> > +
> >  /*
> >   * Mode of operation configurable with kvm-arm.mode early param.
> >   * See Documentation/admin-guide/kernel-parameters.txt for more information.
> > @@ -84,6 +89,7 @@ int __init kvm_arm_init_sve(void);
> >  u32 __attribute_const__ kvm_target_cpu(void);
> >  void kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> >  void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu);
> > +void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu);
> > 
> >  struct kvm_hyp_memcache {
> >  	phys_addr_t head;
> > @@ -405,6 +411,8 @@ struct kvm_arch {
> >  	 * the associated pKVM instance in the hypervisor.
> >  	 */
> >  	struct kvm_protected_vm pkvm;
> > +
> > +	bool enable_hdbss;
> >  };
> > 
> >  struct kvm_vcpu_fault_info {
> > @@ -816,6 +824,12 @@ struct vcpu_reset_state {
> >  	bool		reset;
> >  };
> > 
> > +struct vcpu_hdbss_state {
> > +	phys_addr_t base_phys;
> > +	u32 size;
> > +	u32 next_index;
> > +};
> > +
> >  struct vncr_tlb;
> > 
> >  struct kvm_vcpu_arch {
> > @@ -920,6 +934,9 @@ struct kvm_vcpu_arch {
> > 
> >  	/* Per-vcpu TLB for VNCR_EL2 -- NULL when !NV */
> >  	struct vncr_tlb	*vncr_tlb;
> > +
> > +	/* HDBSS registers info */
> > +	struct vcpu_hdbss_state hdbss;
> >  };
> > 
> >  /*
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index d968aca0461a..3fea8cfe8869 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -183,6 +183,7 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> > 
> >  int kvm_handle_guest_sea(struct kvm_vcpu *vcpu);
> >  int kvm_handle_guest_abort(struct kvm_vcpu *vcpu);
> > +void kvm_flush_hdbss_buffer(struct kvm_vcpu *vcpu);
> > 
> >  phys_addr_t kvm_mmu_get_httbr(void);
> >  phys_addr_t kvm_get_idmap_vector(void);
> > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > index f4436ecc630c..d11f4d0dd4e7 100644
> > --- a/arch/arm64/include/asm/sysreg.h
> > +++ b/arch/arm64/include/asm/sysreg.h
> > @@ -1039,6 +1039,17 @@
> > 
> >  #define GCS_CAP(x)	((((unsigned long)x) & GCS_CAP_ADDR_MASK) | \
> >  					       GCS_CAP_VALID_TOKEN)
> > +
> > +/*
> > + * Definitions for the HDBSS feature
> > + */
> > +#define HDBSS_MAX_SIZE		HDBSSBR_EL2_SZ_2MB
> > +
> > +#define HDBSSBR_EL2(baddr, sz)	(((baddr) & GENMASK(55, 12 + sz)) | \
> > +				 FIELD_PREP(HDBSSBR_EL2_SZ_MASK, sz))
> > +
> > +#define HDBSSPROD_IDX(prod)	FIELD_GET(HDBSSPROD_EL2_INDEX_MASK, prod)
> > +
> >  /*
> >   * Definitions for GICv5 instructions]
> >   */
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 29f0326f7e00..d64da05e25c4 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -125,6 +125,87 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
> >  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> >  }
> > 
> > +void kvm_arm_vcpu_free_hdbss(struct kvm_vcpu *vcpu)
> > +{
> > +	struct page *hdbss_pg;
> > +
> > +	hdbss_pg = phys_to_page(vcpu->arch.hdbss.base_phys);
> > +	if (hdbss_pg)
> > +		__free_pages(hdbss_pg, vcpu->arch.hdbss.size);
> > +
> > +	vcpu->arch.hdbss.size = 0;
> > +}
> > +
> > +static int kvm_cap_arm_enable_hdbss(struct kvm *kvm,
> > +				    struct kvm_enable_cap *cap)
> > +{
> > +	unsigned long i;
> > +	struct kvm_vcpu *vcpu;
> > +	struct page *hdbss_pg = NULL;
> > +	__u64 size = cap->args[0];
> > +	bool enable = cap->args[1] ? true : false;
> > +
> > +	if (!system_supports_hdbss())
> > +		return -EINVAL;
> > +
> > +	if (size > HDBSS_MAX_SIZE)
> > +		return -EINVAL;
> > +
> > +	if (!enable && !kvm->arch.enable_hdbss) /* Already Off */
> > +		return 0;
> > +
> > +	if (enable && kvm->arch.enable_hdbss) /* Already On, can't set size */
> > +		return -EINVAL;
> > +
> > +	if (!enable) { /* Turn it off */
> > +		kvm->arch.mmu.vtcr &= ~(VTCR_EL2_HD | VTCR_EL2_HDBSS | VTCR_EL2_HA);
> > +
> > +		kvm_for_each_vcpu(i, vcpu, kvm) {
> > +			/* Kick vcpus to flush hdbss buffer. */
> > +			kvm_vcpu_kick(vcpu);
> > +
> > +			kvm_arm_vcpu_free_hdbss(vcpu);
> > +		}
> > +
> > +		kvm->arch.enable_hdbss = false;
> > +
> > +		return 0;
> > +	}
> > +
> > +	/* Turn it on */
> > +	kvm_for_each_vcpu(i, vcpu, kvm) {
> > +		hdbss_pg = alloc_pages(GFP_KERNEL_ACCOUNT, size);
> > +		if (!hdbss_pg)
> > +			goto error_alloc;
> > +
> > +		vcpu->arch.hdbss = (struct vcpu_hdbss_state) {
> > +			.base_phys = page_to_phys(hdbss_pg),
> > +			.size = size,
> > +			.next_index = 0,
> > +		};
> > +	}
> > +
> > +	kvm->arch.enable_hdbss = true;
> > +	kvm->arch.mmu.vtcr |= VTCR_EL2_HD | VTCR_EL2_HDBSS | VTCR_EL2_HA;
> > +
> > +	/*
> > +	 * We should kick vcpus out of guest mode here to load new
> > +	 * vtcr value to vtcr_el2 register when re-enter guest mode.
> > +	 */
> > +	kvm_for_each_vcpu(i, vcpu, kvm)
> > +		kvm_vcpu_kick(vcpu);
> > +
> > +	return 0;
> > +
> > +error_alloc:
> > +	kvm_for_each_vcpu(i, vcpu, kvm) {
> > +		if (vcpu->arch.hdbss.base_phys)
> > +			kvm_arm_vcpu_free_hdbss(vcpu);
> > +	}
> > +
> > +	return -ENOMEM;
> > +}
> > +
> >  int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >  			    struct kvm_enable_cap *cap)
> >  {
> > @@ -182,6 +263,11 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >  		r = 0;
> >  		set_bit(KVM_ARCH_FLAG_EXIT_SEA, &kvm->arch.flags);
> >  		break;
> > +	case KVM_CAP_ARM_HW_DIRTY_STATE_TRACK:
> > +		mutex_lock(&kvm->lock);
> > +		r = kvm_cap_arm_enable_hdbss(kvm, cap);
> > +		mutex_unlock(&kvm->lock);
> > +		break;
> >  	default:
> >  		break;
> >  	}
> > @@ -471,6 +557,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >  			r = kvm_supports_cacheable_pfnmap();
> >  		break;
> > 
> > +	case KVM_CAP_ARM_HW_DIRTY_STATE_TRACK:
> > +		r = system_supports_hdbss();
> > +		break;
> >  	default:
> >  		r = 0;
> >  	}
> > @@ -1120,6 +1209,9 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
> >  		if (kvm_dirty_ring_check_request(vcpu))
> >  			return 0;
> > 
> > +		if (kvm_check_request(KVM_REQ_FLUSH_HDBSS, vcpu))
> > +			kvm_flush_hdbss_buffer(vcpu);
> > +
> >  		check_nested_vcpu_requests(vcpu);
> >  	}
> > 
> > @@ -1898,7 +1990,17 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl,
> > 
> >  void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
> >  {
> > +	/*
> > +	 * Flush all CPUs' dirty log buffers to the dirty_bitmap.  Called
> > +	 * before reporting dirty_bitmap to userspace. Send a request with
> > +	 * KVM_REQUEST_WAIT to flush buffer synchronously.
> > +	 */
> > +	struct kvm_vcpu *vcpu;
> > +
> > +	if (!kvm->arch.enable_hdbss)
> > +		return;
> > 
> > +	kvm_make_all_cpus_request(kvm, KVM_REQ_FLUSH_HDBSS);
> >  }
> > 
> >  static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index 9db3f11a4754..600cbc4f8ae9 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -213,6 +213,23 @@ static void __vcpu_put_deactivate_traps(struct kvm_vcpu *vcpu)
> >  	local_irq_restore(flags);
> >  }
> > 
> > +static void __load_hdbss(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm *kvm = vcpu->kvm;
> > +	u64 br_el2, prod_el2;
> > +
> > +	if (!kvm->arch.enable_hdbss)
> > +		return;
> > +
> > +	br_el2 = HDBSSBR_EL2(vcpu->arch.hdbss.base_phys, vcpu->arch.hdbss.size);
> > +	prod_el2 = vcpu->arch.hdbss.next_index;
> > +
> > +	write_sysreg_s(br_el2, SYS_HDBSSBR_EL2);
> > +	write_sysreg_s(prod_el2, SYS_HDBSSPROD_EL2);
> > +
> > +	isb();
> > +}
> > +
> 
> I see in the code below you trust that the tracking will happen with 
> PAGE_SIZE granularity (you track with PAGE_SHIFT).

Oh, that was misleading :\
The mentioned routine is not wrong AFAICS, but without the patch I sent, if 
the VMM does not manually set eager splitting, and is using some sort of 
hugepages (even transparent, without noticing) it could end up sending just 
PAGE_SIZE instead of the whole hugepage size, which breaks migration. 

Does that make sense?

Thanks!
Leo

> 
> That may be a problem when we have guest memory backed by hugepages or 
> transparent huge pages. 
> 
> When we are using HDBSS, there is no fault happening, so we have no way of 
> doing on-demand block splitting, so we need to make use of eager block 
> splitting, _before_ we start to track anything, or else we may have 
> different-sized pages in the HDBSS buffer, which is harder to deal with.
> 
> Suggestion: do the eager splitting before we enable HDBSS. 
> 
> For this to happen, we have to enable the EAGER_SPLIT_CHUNK_SIZE 
> capability, which can only be enabled when all memslots are empty.
> 
> I suggest doing that at kvm_init_stage2_mmu(), and checking if HDBSS is 
> in which case we set mmu->split_page_chunk_size to PAGESIZE.
> 
> I will send a patch you can put before this one to make sure it works :)
> 
> Thanks!
> Leo
> 
> 
> >  void kvm_vcpu_load_vhe(struct kvm_vcpu *vcpu)
> >  {
> >  	host_data_ptr(host_ctxt)->__hyp_running_vcpu = vcpu;
> > @@ -220,10 +237,12 @@ void kvm_vcpu_load_vhe(struct kvm_vcpu *vcpu)
> >  	__vcpu_load_switch_sysregs(vcpu);
> >  	__vcpu_load_activate_traps(vcpu);
> >  	__load_stage2(vcpu->arch.hw_mmu, vcpu->arch.hw_mmu->arch);
> > +	__load_hdbss(vcpu);
> >  }
> > 
> >  void kvm_vcpu_put_vhe(struct kvm_vcpu *vcpu)
> >  {
> > +	kvm_flush_hdbss_buffer(vcpu);
> >  	__vcpu_put_deactivate_traps(vcpu);
> >  	__vcpu_put_switch_sysregs(vcpu);
> > 
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 070a01e53fcb..42b0710a16ce 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1896,6 +1896,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >  	if (writable)
> >  		prot |= KVM_PGTABLE_PROT_W;
> > 
> > +	if (writable && kvm->arch.enable_hdbss && logging_active)
> > +		prot |= KVM_PGTABLE_PROT_DBM;
> > +
> >  	if (exec_fault)
> >  		prot |= KVM_PGTABLE_PROT_X;
> > 
> > @@ -2033,6 +2036,70 @@ int kvm_handle_guest_sea(struct kvm_vcpu *vcpu)
> >  	return 0;
> >  }
> > 
> > +void kvm_flush_hdbss_buffer(struct kvm_vcpu *vcpu)
> > +{
> > +	int idx, curr_idx;
> > +	u64 br_el2;
> > +	u64 *hdbss_buf;
> > +	struct kvm *kvm = vcpu->kvm;
> > +
> > +	if (!kvm->arch.enable_hdbss)
> > +		return;
> > +
> > +	curr_idx = HDBSSPROD_IDX(read_sysreg_s(SYS_HDBSSPROD_EL2));
> > +	br_el2 = HDBSSBR_EL2(vcpu->arch.hdbss.base_phys, vcpu->arch.hdbss.size);
> > +
> > +	/* Do nothing if HDBSS buffer is empty or br_el2 is NULL */
> > +	if (curr_idx == 0 || br_el2 == 0)
> > +		return;
> > +
> > +	hdbss_buf = page_address(phys_to_page(vcpu->arch.hdbss.base_phys));
> > +	if (!hdbss_buf)
> > +		return;
> > +
> > +	guard(write_lock_irqsave)(&vcpu->kvm->mmu_lock);
> > +	for (idx = 0; idx < curr_idx; idx++) {
> > +		u64 gpa;
> > +
> > +		gpa = hdbss_buf[idx];
> > +		if (!(gpa & HDBSS_ENTRY_VALID))
> > +			continue;
> > +
> > +		gpa &= HDBSS_ENTRY_IPA;
> > +		kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
> > +	}
> 
> Here ^
> 
> > +
> > +	/* reset HDBSS index */
> > +	write_sysreg_s(0, SYS_HDBSSPROD_EL2);
> > +	vcpu->arch.hdbss.next_index = 0;
> > +	isb();
> > +}
> > +
> > +static int kvm_handle_hdbss_fault(struct kvm_vcpu *vcpu)
> > +{
> > +	u64 prod;
> > +	u64 fsc;
> > +
> > +	prod = read_sysreg_s(SYS_HDBSSPROD_EL2);
> > +	fsc = FIELD_GET(HDBSSPROD_EL2_FSC_MASK, prod);
> > +
> > +	switch (fsc) {
> > +	case HDBSSPROD_EL2_FSC_OK:
> > +		/* Buffer full, which is reported as permission fault. */
> > +		kvm_flush_hdbss_buffer(vcpu);
> > +		return 1;
> > +	case HDBSSPROD_EL2_FSC_ExternalAbort:
> > +	case HDBSSPROD_EL2_FSC_GPF:
> > +		return -EFAULT;
> > +	default:
> > +		/* Unknown fault. */
> > +		WARN_ONCE(1,
> > +				"Unexpected HDBSS fault type, FSC: 0x%llx (prod=0x%llx, vcpu=%d)\n",
> > +				fsc, prod, vcpu->vcpu_id);
> > +		return -EFAULT;
> > +	}
> > +}
> > +
> >  /**
> >   * kvm_handle_guest_abort - handles all 2nd stage aborts
> >   * @vcpu:	the VCPU pointer
> > @@ -2071,6 +2138,9 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
> > 
> >  	is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
> > 
> > +	if (esr_iss2_is_hdbssf(esr))
> > +		return kvm_handle_hdbss_fault(vcpu);
> > +
> >  	if (esr_fsc_is_translation_fault(esr)) {
> >  		/* Beyond sanitised PARange (which is the IPA limit) */
> >  		if (fault_ipa >= BIT_ULL(get_kvm_ipa_limit())) {
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index 959532422d3a..c03a4b310b53 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -161,6 +161,9 @@ void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
> >  	free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> >  	kfree(vcpu->arch.vncr_tlb);
> >  	kfree(vcpu->arch.ccsidr);
> > +
> > +	if (vcpu->kvm->arch.enable_hdbss)
> > +		kvm_arm_vcpu_free_hdbss(vcpu);
> >  }
> > 
> >  static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
> > --
> > 2.33.0
> > 

^ permalink raw reply

* Re: [PATCH v2] bootconfig: Apply early options from embedded config
From: Masami Hiramatsu @ 2026-03-26 14:30 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Breno Leitao, Jonathan Corbet, Shuah Khan, linux-kernel,
	linux-trace-kernel, linux-doc, oss, paulmck, rostedt, kernel-team
In-Reply-To: <20260325232204.05edbb21c7602b6408ca007b@kernel.org>

On Wed, 25 Mar 2026 23:22:04 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > +	/*
> > +	 * Keys that do not match any early_param() handler are silently
> > +	 * ignored — do_early_param() always returns 0.
> > +	 */
> > +	xbc_node_for_each_key_value(root, knode, val) {
> 
> [sashiko comment]
> | Does this loop handle array values correctly?
> | xbc_node_for_each_key_value() only assigns the first value of an array to
> | the val pointer before advancing to the next key. It does not iterate over
> | the child nodes of the array.
> | If the bootconfig contains a multi-value key like
> | kernel.console = "ttyS0", "tty0", will the subsequent values in the array
> | be silently dropped instead of passed to the early_param handlers?
> 
> Also, good catch :) we need to use xbc_node_for_each_array_value()
> for inner loop.

FYI, xbc_snprint_cmdline() translates the arraied parameter as
multiple parameters. For example,

foo = bar, buz;

will be converted to

foo=bar foo=buz

Thus, I think we should do the same thing below;

> 
> > +		if (xbc_node_compose_key_after(root, knode, xbc_namebuf, XBC_KEYLEN_MAX) < 0)
> > +			continue;
> > +
> > +		/*
> > +		 * We need to copy const char *val to a char pointer,
> > +		 * which is what do_early_param() need, given it might
> > +		 * call strsep(), strtok() later.
> > +		 */
> > +		ret = strscpy(val_buf, val, sizeof(val_buf));
> > +		if (ret < 0) {
> > +			pr_warn("ignoring bootconfig value '%s', too long\n",
> > +				xbc_namebuf);
> > +			continue;
> > +		}
> > +		do_early_param(xbc_namebuf, val_buf, NULL, NULL);

So instead of this;

xbc_array_for_each_value(vnode, val) {
	do_early_param(xbc_namebuf, val, NULL, NULL);
}

Maybe it is a good timing to recondier unifying kernel cmdline and bootconfig
from API viewpoint.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply

* [PATCH v2] doc: Add CPU Isolation documentation
From: Frederic Weisbecker @ 2026-03-26 14:00 UTC (permalink / raw)
  To: LKML
  Cc: Frederic Weisbecker, Anna-Maria Behnsen, Gabriele Monaco,
	Ingo Molnar, Jonathan Corbet, Marcelo Tosatti, Marco Crivellari,
	Michal Hocko, Paul E . McKenney, Peter Zijlstra, Phil Auld,
	Steven Rostedt, Thomas Gleixner, Valentin Schneider,
	Vlastimil Babka, Waiman Long, linux-doc,
	Sebastian Andrzej Siewior, Bagas Sanjaya

nohz_full was introduced in v3.10 in 2013, which means this
documentation is overdue for 13 years.

Fortunately Paul wrote a part of the needed documentation a while ago,
especially concerning nohz_full in Documentation/timers/no_hz.rst and
also about per-CPU kthreads in
Documentation/admin-guide/kernel-per-CPU-kthreads.rst

Introduce a new page that gives an overview of CPU isolation in general.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
v2:
   - Fix links and code blocks (Bagas and Sebastian)
   - Isolation is not only about userspace, rephrase accordingly (Valentin)
   - Paste BIOS issues suggestion from Valentin
   - Include the whole rtla suite (Valentin)
   - Rephrase a few details (Waiman)
   - Talk about RCU induced overhead rather than slower RCU (Sebastian)

 Documentation/admin-guide/cpu-isolation.rst | 357 ++++++++++++++++++++
 Documentation/admin-guide/index.rst         |   1 +
 2 files changed, 358 insertions(+)
 create mode 100644 Documentation/admin-guide/cpu-isolation.rst

diff --git a/Documentation/admin-guide/cpu-isolation.rst b/Documentation/admin-guide/cpu-isolation.rst
new file mode 100644
index 000000000000..886dec79b056
--- /dev/null
+++ b/Documentation/admin-guide/cpu-isolation.rst
@@ -0,0 +1,357 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+CPU Isolation
+=============
+
+Introduction
+============
+
+"CPU Isolation" means leaving a CPU exclusive to a given workload
+without any undesired code interference from the kernel.
+
+Those interferences, commonly pointed out as "noise", can be triggered
+by asynchronous events (interrupts, timers, scheduler preemption by
+workqueues and kthreads, ...) or synchronous events (syscalls and page
+faults).
+
+Such noise usually goes unnoticed. After all synchronous events are a
+component of the requested kernel service. And asynchronous events are
+either sufficiently well distributed by the scheduler when executed
+as tasks or reasonably fast when executed as interrupt. The timer
+interrupt can even execute 1024 times per seconds without a significant
+and measurable impact most of the time.
+
+However some rare and extreme workloads can be quite sensitive to
+those kinds of noise. This is the case, for example, with high
+bandwidth network processing that can't afford losing a single packet
+or very low latency network processing. Typically those usecases
+involve DPDK, bypassing the kernel networking stack and performing
+direct access to the networking device from userscace.
+
+In order to run a CPU without or with limited kernel noise, the
+related housekeeping work needs to be either shutdown, migrated or
+offloaded.
+
+Housekeeping
+============
+
+In the CPU isolation terminology, housekeeping is the work, often
+asynchronous, that the kernel needs to process in order to maintain
+all its services. It matches the noises and disturbances enumerated
+above except when at least one CPU is isolated. Then housekeeping may
+make use of further coping mechanisms if CPU-tied work must be
+offloaded.
+
+Housekeeping CPUs are the non-isolated CPUs where the kernel noise
+is moved away from isolated CPUs.
+
+The isolation can be implemented in several ways depending on the
+nature of the noise:
+
+- Unbound work, where "unbound" means not tied to any CPU, can be
+  simply migrated away from isolated CPUs to housekeeping CPUs.
+  This is the case of unbound workqueues, kthreads and timers.
+
+- Bound work, where "bound" means tied to a specific CPU, usually
+  can't be moved away as-is by nature. Either:
+
+	- The work must switch to a locked implementation. Eg: This is
+	  the case of RCU with CONFIG_RCU_NOCB_CPU.
+
+	- The related feature must be shutdown and considered
+	  incompatible with isolated CPUs. Eg: Lockup watchdog,
+	  unreliable clocksources, etc...
+
+	- An elaborated and heavyweight coping mechanism stands as a
+	  replacement. Eg: the timer tick is shutdown on nohz_full but
+	  with the constraint of running a single task on the CPU. A
+	  significant cost penalty is added on kernel entry/exit and
+	  a residual 1Hz scheduler tick is offloaded to housekeeping
+	  CPUs.
+
+In any case, housekeeping work has to be handled, which is why there
+must be at least one housekeeping CPU in the system, preferrably more
+if the machine runs a lot of CPUs. For example one per node on NUMA
+systems.
+
+Also CPU isolation often means a tradeoff between noise-free isolated
+CPUs and added overhead on housekeeping CPUs, sometimes even on
+isolated CPUs entering the kernel.
+
+Isolation features
+==================
+
+Different levels of isolation can be configured in the kernel, each of
+which having their own drawbacks and tradeoffs.
+
+Scheduler domain isolation
+--------------------------
+
+This feature isolates a CPU from the scheduler topology. As a result,
+the target isn't part of the load balancing. Tasks won't migrate
+neither from nor to it unless affined explicitly.
+
+As a side effect the CPU is also isolated from unbound workqueues and
+unbound kthreads.
+
+Requirements
+~~~~~~~~~~~~
+
+- CONFIG_CPUSETS=y for the cpusets based interface
+
+Tradeoffs
+~~~~~~~~~
+
+By nature, the system load is overall less distributed since some CPUs
+are extracted from the global load balancing.
+
+Interface
+~~~~~~~~~
+
+- Documentation/admin-guide/cgroup-v2.rst cpuset isolated partitions are recommended
+  because they are tunable at runtime.
+
+- The 'isolcpus=' kernel boot parameter with the 'domain' flag is a
+  less flexible alternative that doesn't allow for runtime
+  reconfiguration.
+
+IRQs isolation
+--------------
+
+Isolate the IRQs whenever possible, so that they don't fire on the
+target CPUs.
+
+Interface
+~~~~~~~~~
+
+- The file /proc/irq/\*/smp_affinity as explained in detail in
+  Documentation/core-api/irq/irq-affinity.rst page.
+
+- The "irqaffinity=" kernel boot parameter for a default setting.
+
+- The "managed_irq" flag in the "isolcpus=" kernel boot parameter
+  tries a best effort affinity override for managed IRQs.
+
+Full Dynticks (aka nohz_full)
+-----------------------------
+
+Full dynticks extends the dynticks idle mode, which stop the tick when
+the CPU is idle, to CPUs running a single task in userspace. That is,
+the timer tick is stopped if the environment allows it.
+
+Global timer callbacks are also isolated from the nohz_full CPUs.
+
+Requirements
+~~~~~~~~~~~~
+
+- CONFIG_NO_HZ_FULL=y
+
+Constraints
+~~~~~~~~~~~
+
+- The isolated CPUs must run a single task only. Multitask requires
+  the tick to maintain preemption. This is usually fine since the
+  workload usually can't stand the latency of random context switches.
+
+- No call to the kernel from isolated CPUs, at the risk of triggering
+  random noise.
+
+- No use of posix CPU timers on isolated CPUs.
+
+- Architecture must have a stable and reliable clocksource (no
+  unreliable TSC that requires the watchdog).
+
+
+Tradeoffs
+~~~~~~~~~
+
+In terms of cost, this is the most invasive isolation feature. It is
+assumed to be used when the workload spends most of its time in
+userspace and doesn't rely on the kernel except for preparatory
+work because:
+
+- RCU adds more overhead due to the locked, offloaded and threaded
+  callbacks processing (the same that would be obtained with "rcu_nocb"
+  boot parameter).
+
+- Kernel entry/exit through syscalls, exceptions and IRQs are more
+  costly due to fully ordered RmW operations that maintain userspace
+  as RCU extended quiescent state. Also the CPU time is accounted on
+  kernel boundaries instead of periodically from the tick.
+
+- Housekeeping CPUs must run a 1Hz residual remote scheduler tick
+  on behalf of the isolated CPUs.
+
+Checklist
+=========
+
+You have set up each of the above isolation features but you still
+observe jitters that trash your workload? Make sure to check a few
+elements before proceeding.
+
+Some of these checklist items are similar to those of real time
+workloads:
+
+- Use mlock() to prevent your pages from being swapped away. Page
+  faults are usually not compatible with jitter sensitive workloads.
+
+- Avoid SMT to prevent your hardware thread from being "preempted"
+  by another one.
+
+- CPU frequency changes may induce subtle sorts of jitter in a
+  workload. Cpufreq should be used and tuned with caution.
+
+- Deep C-states may result in latency issues upon wake-up. If this
+  happens to be a problem, C-states can be limited via kernel boot
+  parameters such as processor.max_cstate or intel_idle.max_cstate.
+  More finegrained tunings are described in
+  Documentation/admin-guide/pm/cpuidle.rst page
+
+- Your system may be subject to firmware-originating interrupts - x86 has
+  System Management Interrupts (SMIs) for example. Check your system BIOS
+  to disable such interference, and with some luck your vendor will have
+  a BIOS tuning guidance for low-latency operations.
+
+
+Full isolation example
+======================
+
+In this example, the system has 8 CPUs and the 8th is to be fully
+isolated. Since CPUs start from 0, the 8th CPU is CPU 7.
+
+Kernel parameters
+-----------------
+
+Set the following kernel boot parameters to disable SMT and setup tick
+and IRQ isolation:
+
+- Full dynticks: nohz_full=7
+
+- IRQs isolation: irqaffinity=0-6
+
+- Managed IRQs isolation: isolcpus=managed_irq,7
+
+- Prevent from SMT: nosmt
+
+The full command line is then:
+
+  nohz_full=7 irqaffinity=0-6 isolcpus=managed_irq,7 nosmt
+
+CPUSET configuration (cgroup v2)
+--------------------------------
+
+Assuming cgroup v2 is mounted to /sys/fs/cgroup, the following script
+isolates CPU 7 from scheduler domains.
+
+::
+
+  cd /sys/fs/cgroup
+  # Activate the cpuset subsystem
+  echo +cpuset > cgroup.subtree_control
+  # Create partition to be isolated
+  mkdir test
+  cd test
+  echo +cpuset > cgroup.subtree_control
+  # Isolate CPU 7
+  echo 7 > cpuset.cpus
+  echo "isolated" > cpuset.cpus.partition
+
+The userspace workload
+----------------------
+
+Fake a pure userspace workload, the below program runs a dummy
+userspace loop on the isolated CPU 7.
+
+::
+
+  #include <stdio.h>
+  #include <fcntl.h>
+  #include <unistd.h>
+  #include <errno.h>
+  int main(void)
+  {
+      // Move the current task to the isolated cpuset (bind to CPU 7)
+      int fd = open("/sys/fs/cgroup/test/cgroup.procs", O_WRONLY);
+      if (fd < 0) {
+          perror("Can't open cpuset file...\n");
+          return 0;
+      }
+
+      write(fd, "0\n", 2);
+      close(fd);
+
+      // Run an endless dummy loop until the launcher kills us
+      while (1)
+      ;
+
+      return 0;
+  }
+
+Build it and save for later step:
+
+::
+
+  # gcc user_loop.c -o user_loop
+
+The launcher
+------------
+
+The below launcher runs the above program for 10 seconds and traces
+the noise resulting from preempting tasks and IRQs.
+
+::
+
+  TRACING=/sys/kernel/tracing/
+  # Make sure tracing is off for now
+  echo 0 > $TRACING/tracing_on
+  # Flush previous traces
+  echo > $TRACING/trace
+  # Record disturbance from other tasks
+  echo 1 > $TRACING/events/sched/sched_switch/enable
+  # Record disturbance from interrupts
+  echo 1 > $TRACING/events/irq_vectors/enable
+  # Now we can start tracing
+  echo 1 > $TRACING/tracing_on
+  # Run the dummy user_loop for 10 seconds on CPU 7
+  ./user_loop &
+  USER_LOOP_PID=$!
+  sleep 10
+  kill $USER_LOOP_PID
+  # Disable tracing and save traces from CPU 7 in a file
+  echo 0 > $TRACING/tracing_on
+  cat $TRACING/per_cpu/cpu7/trace > trace.7
+
+If no specific problem arose, the output of trace.7 should look like
+the following:
+
+::
+
+  <idle>-0 [007] d..2. 1980.976624: sched_switch: prev_comm=swapper/7 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=user_loop next_pid=1553 next_prio=120
+  user_loop-1553 [007] d.h.. 1990.946593: reschedule_entry: vector=253
+  user_loop-1553 [007] d.h.. 1990.946593: reschedule_exit: vector=253
+
+That is, no specific noise triggered between the first trace and the
+second during 10 seconds when user_loop was running.
+
+Debugging
+=========
+
+Of course things are never so easy, especially on this matter.
+Chances are that actual noise will be observed in the aforementioned
+trace.7 file.
+
+The best way to investigate further is to enable finer grained
+tracepoints such as those of subsystems producing asynchronous
+events: workqueue, timer, irq_vector, etc... It also can be
+interesting to enable the tick_stop event to diagnose why the tick is
+retained when that happens.
+
+Some tools may also be useful for higher level analysis:
+
+- Documentation/tools/rtla/rtla.rst provides a suite of tools to analyze
+  latency and noise in the system. For example Documentation/tools/rtla/rtla-osnoise.rst
+  runs a kernel tracer that analyzes and output a summary of the noises.
+
+- dynticks-testing does something similar to rtla-osnoise but in userspace. It is available
+  at git://git.kernel.org/pub/scm/linux/kernel/git/frederic/dynticks-testing.git
diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
index b734f8a2a2c4..cd28dfe91b06 100644
--- a/Documentation/admin-guide/index.rst
+++ b/Documentation/admin-guide/index.rst
@@ -94,6 +94,7 @@ likely to be of interest on almost any system.
 
    cgroup-v2
    cgroup-v1/index
+   cpu-isolation
    cpu-load
    mm/index
    module-signing
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Dave Stevenson @ 2026-03-26 13:39 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Nicolas Frattaroli, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
	Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
	Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
	linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
	intel-xe, linux-doc, Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <acUi6NEPJ0p48a3U@intel.com>

On Thu, 26 Mar 2026 at 12:13, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Thu, Mar 26, 2026 at 11:16:12AM +0000, Dave Stevenson wrote:
> > On Wed, 25 Mar 2026 at 13:43, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Wed, Mar 25, 2026 at 12:49:19PM +0000, Dave Stevenson wrote:
> > > > On Tue, 24 Mar 2026 at 16:02, Nicolas Frattaroli
> > > > <nicolas.frattaroli@collabora.com> wrote:
> > > > >
> > > > > Add a new general DRM property named "color format" which can be used by
> > > > > userspace to request the display driver to output a particular color
> > > > > format.
> > > > >
> > > > > Possible options are:
> > > > >     - auto (setup by default, driver internally picks the color format)
> > > > >     - rgb
> > > > >     - ycbcr444
> > > > >     - ycbcr422
> > > > >     - ycbcr420
> > > > >
> > > > > Drivers should advertise from this list which formats they support.
> > > > > Together with this list and EDID data from the sink we should be able
> > > > > to relay a list of usable color formats to users to pick from.
> > > > >
> > > > > Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
> > > > > Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> > > > > Co-developed-by: Andri Yngvason <andri@yngvason.is>
> > > > > Signed-off-by: Andri Yngvason <andri@yngvason.is>
> > > > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > > > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic_helper.c |   5 ++
> > > > >  drivers/gpu/drm/drm_atomic_uapi.c   |  11 ++++
> > > > >  drivers/gpu/drm/drm_connector.c     | 108 ++++++++++++++++++++++++++++++++++++
> > > > >  include/drm/drm_connector.h         | 104 ++++++++++++++++++++++++++++++++++
> > > > >  4 files changed, 228 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > index 26953ed6b53e..b7753454b777 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > @@ -737,6 +737,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > > >                         if (old_connector_state->max_requested_bpc !=
> > > > >                             new_connector_state->max_requested_bpc)
> > > > >                                 new_crtc_state->connectors_changed = true;
> > > > > +
> > > > > +                       if (old_connector_state->color_format !=
> > > > > +                           new_connector_state->color_format)
> > > > > +                               new_crtc_state->connectors_changed = true;
> > > > > +
> > > > >                 }
> > > > >
> > > > >                 if (funcs->atomic_check)
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index 5bd5bf6661df..dee510c85e59 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -935,6 +935,15 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > >                 state->privacy_screen_sw_state = val;
> > > > >         } else if (property == connector->broadcast_rgb_property) {
> > > > >                 state->hdmi.broadcast_rgb = val;
> > > > > +       } else if (property == connector->color_format_property) {
> > > > > +               if (val > INT_MAX || !drm_connector_color_format_valid(val)) {
> > > > > +                       drm_dbg_atomic(connector->dev,
> > > > > +                                      "[CONNECTOR:%d:%s] unknown color format %llu\n",
> > > > > +                                      connector->base.id, connector->name, val);
> > > > > +                       return -EINVAL;
> > > > > +               }
> > > > > +
> > > > > +               state->color_format = val;
> > > > >         } else if (connector->funcs->atomic_set_property) {
> > > > >                 return connector->funcs->atomic_set_property(connector,
> > > > >                                 state, property, val);
> > > > > @@ -1020,6 +1029,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > > > >                 *val = state->privacy_screen_sw_state;
> > > > >         } else if (property == connector->broadcast_rgb_property) {
> > > > >                 *val = state->hdmi.broadcast_rgb;
> > > > > +       } else if (property == connector->color_format_property) {
> > > > > +               *val = state->color_format;
> > > > >         } else if (connector->funcs->atomic_get_property) {
> > > > >                 return connector->funcs->atomic_get_property(connector,
> > > > >                                 state, property, val);
> > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > > index 47dc53c4a738..e848374dee0b 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > @@ -1388,6 +1388,18 @@ static const u32 hdmi_colorspaces =
> > > > >         BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) |
> > > > >         BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER);
> > > > >
> > > > > +static const u32 hdmi_colorformats =
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
> > > > > +
> > > > > +static const u32 dp_colorformats =
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
> > > > > +       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
> > > > > +
> > > > >  /*
> > > > >   * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel Encoding/Colorimetry
> > > > >   * Format Table 2-120
> > > > > @@ -2940,6 +2952,102 @@ int drm_connector_attach_colorspace_property(struct drm_connector *connector)
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_connector_attach_colorspace_property);
> > > > >
> > > > > +/**
> > > > > + * drm_connector_attach_color_format_property - create and attach color format property
> > > > > + * @connector: connector to create the color format property on
> > > > > + * @supported_color_formats: bitmask of bit-shifted &enum drm_output_color_format
> > > > > + *                           values the connector supports
> > > > > + *
> > > > > + * Called by a driver to create a color format property. The property is
> > > > > + * attached to the connector automatically on success.
> > > > > + *
> > > > > + * @supported_color_formats should only include color formats the connector
> > > > > + * type can actually support.
> > > > > + *
> > > > > + * Returns:
> > > > > + * 0 on success, negative errno on error
> > > > > + */
> > > > > +int drm_connector_attach_color_format_property(struct drm_connector *connector,
> > > > > +                                              unsigned long supported_color_formats)
> > > > > +{
> > > > > +       struct drm_device *dev = connector->dev;
> > > > > +       struct drm_prop_enum_list enum_list[DRM_CONNECTOR_COLOR_FORMAT_COUNT];
> > > > > +       unsigned int i = 0;
> > > > > +       unsigned long fmt;
> > > > > +
> > > > > +       if (connector->color_format_property)
> > > > > +               return 0;
> > > > > +
> > > > > +       if (!supported_color_formats) {
> > > > > +               drm_err(dev, "No supported color formats provided on [CONNECTOR:%d:%s]\n",
> > > > > +                       connector->base.id, connector->name);
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +
> > > > > +       if (supported_color_formats & ~GENMASK(DRM_OUTPUT_COLOR_FORMAT_COUNT - 1, 0)) {
> > > > > +               drm_err(dev, "Unknown color formats provided on [CONNECTOR:%d:%s]\n",
> > > > > +                       connector->base.id, connector->name);
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +
> > > > > +       switch (connector->connector_type) {
> > > > > +       case DRM_MODE_CONNECTOR_HDMIA:
> > > > > +       case DRM_MODE_CONNECTOR_HDMIB:
> > > > > +               if (supported_color_formats & ~hdmi_colorformats) {
> > > > > +                       drm_err(dev, "Color formats not allowed for HDMI on [CONNECTOR:%d:%s]\n",
> > > > > +                               connector->base.id, connector->name);
> > > > > +                       return -EINVAL;
> > > > > +               }
> > > > > +               break;
> > > > > +       case DRM_MODE_CONNECTOR_DisplayPort:
> > > > > +       case DRM_MODE_CONNECTOR_eDP:
> > > > > +               if (supported_color_formats & ~dp_colorformats) {
> > > > > +                       drm_err(dev, "Color formats not allowed for DP on [CONNECTOR:%d:%s]\n",
> > > > > +                               connector->base.id, connector->name);
> > > > > +                       return -EINVAL;
> > > > > +               }
> > > > > +               break;
> > > > > +       }
> > > > > +
> > > > > +       enum_list[0].name = "AUTO";
> > > > > +       enum_list[0].type = DRM_CONNECTOR_COLOR_FORMAT_AUTO;
> > > > > +
> > > > > +       for_each_set_bit(fmt, &supported_color_formats, DRM_OUTPUT_COLOR_FORMAT_COUNT) {
> > > > > +               switch (fmt) {
> > > > > +               case DRM_OUTPUT_COLOR_FORMAT_RGB444:
> > > > > +                       enum_list[++i].type = DRM_CONNECTOR_COLOR_FORMAT_RGB444;
> > > > > +                       break;
> > > > > +               case DRM_OUTPUT_COLOR_FORMAT_YCBCR444:
> > > > > +                       enum_list[++i].type = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444;
> > > > > +                       break;
> > > > > +               case DRM_OUTPUT_COLOR_FORMAT_YCBCR422:
> > > > > +                       enum_list[++i].type = DRM_CONNECTOR_COLOR_FORMAT_YCBCR422;
> > > > > +                       break;
> > > > > +               case DRM_OUTPUT_COLOR_FORMAT_YCBCR420:
> > > > > +                       enum_list[++i].type = DRM_CONNECTOR_COLOR_FORMAT_YCBCR420;
> > > > > +                       break;
> > > > > +               default:
> > > > > +                       drm_warn(dev, "Unknown supported format %ld on [CONNECTOR:%d:%s]\n",
> > > > > +                                fmt, connector->base.id, connector->name);
> > > > > +                       continue;
> > > > > +               }
> > > > > +               enum_list[i].name = drm_hdmi_connector_get_output_format_name(fmt);
> > > > > +       }
> > > > > +
> > > > > +       connector->color_format_property =
> > > > > +               drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, "color format",
> > > > > +                                        enum_list, i + 1);
> > > > > +
> > > > > +       if (!connector->color_format_property)
> > > > > +               return -ENOMEM;
> > > > > +
> > > > > +       drm_object_attach_property(&connector->base, connector->color_format_property,
> > > > > +                                  DRM_CONNECTOR_COLOR_FORMAT_AUTO);
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_connector_attach_color_format_property);
> > > > > +
> > > > >  /**
> > > > >   * drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
> > > > >   * @old_state: old connector state to compare
> > > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > > > index af8b92d2d5b7..bd549f912b76 100644
> > > > > --- a/include/drm/drm_connector.h
> > > > > +++ b/include/drm/drm_connector.h
> > > > > @@ -571,14 +571,102 @@ enum drm_colorspace {
> > > > >   *   YCbCr 4:2:2 output format (ie. with horizontal subsampling)
> > > > >   * @DRM_OUTPUT_COLOR_FORMAT_YCBCR420:
> > > > >   *   YCbCr 4:2:0 output format (ie. with horizontal and vertical subsampling)
> > > > > + * @DRM_OUTPUT_COLOR_FORMAT_COUNT:
> > > > > + *   Number of valid output color format values in this enum
> > > > >   */
> > > > >  enum drm_output_color_format {
> > > > >         DRM_OUTPUT_COLOR_FORMAT_RGB444 = 0,
> > > > >         DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
> > > > >         DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
> > > > >         DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
> > > > > +       DRM_OUTPUT_COLOR_FORMAT_COUNT,
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * enum drm_connector_color_format - Connector Color Format Request
> > > > > + *
> > > > > + * This enum, unlike &enum drm_output_color_format, is used to specify requests
> > > > > + * for a specific color format on a connector through the DRM "color format"
> > > > > + * property. The difference is that it has an "AUTO" value to specify that
> > > > > + * no specific choice has been made.
> > > > > + */
> > > > > +enum drm_connector_color_format {
> > > > > +       /**
> > > > > +        * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > +        * helpers should pick a suitable color format. All implementations of a
> > > > > +        * specific display protocol must behave the same way with "AUTO", but
> > > > > +        * different display protocols do not necessarily have the same "AUTO"
> > > > > +        * semantics.
> > > > > +        *
> > > > > +        * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > +        * bandwidth required for full-scale RGB is not available, or the mode
> > > > > +        * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > +        * YCbCr 4:2:0.
> > > >
> > > > Is there a reason you propose dropping back to YCbCr 4:2:0 without
> > > > trying YCbCr 4:2:2 first? Minimising the subsampling is surely
> > > > beneficial, and vc4 for one can do 4:2:2 but not 4:2:0.
> > >
> > > On HDMI 4:2:2 is always 12bpc, so it doesn't save any bandwidth
> > > compared to 8bpc 4:4:4.
> >
> > It does save bandwidth against 10 or 12bpc RGB 4:4:4.
> >
> > Or is the implication that max_bpc = 12 and
> > DRM_CONNECTOR_COLOR_FORMAT_AUTO should drop bpc down to 8 and select
> > RGB in preference to selecting 4:2:2?
>
> Yeah, YCbCr has all kinds of extra complications compared to RGB, so
> the policy is to use RGB if possible, and only fall back to YCbCr as a
> last resort. And in that case 4:2:0 is the only thing that can help.

So a media player wanting to do 12bpc HDR playback at 4k60 over HDMI
2.0 ends up with 8bpc RGB regardless. That sucks.
I guess at least an override is being added so userspace can take control.

I'd missed that vc4 had its behaviour changed with the
drm_hdmi_state_helper update :-(

  Dave

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox